* [PATCH v4 1/7] plugins: add flag to specify whether PC is rw
2026-02-24 15:46 [PATCH v4 0/7] Enable PC diversion via the plugin API Florian Hofhammer
@ 2026-02-24 15:48 ` Florian Hofhammer
2026-02-24 17:41 ` Alex Bennée
2026-02-24 15:50 ` [PATCH v4 2/7] linux-user: make syscall emulation interruptible Florian Hofhammer
` (6 subsequent siblings)
7 siblings, 1 reply; 46+ messages in thread
From: Florian Hofhammer @ 2026-02-24 15:48 UTC (permalink / raw)
To: qemu-devel
Cc: alex.bennee, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
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.
Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
---
include/plugins/qemu-plugin.h | 3 +++
plugins/api.c | 3 ++-
plugins/core.c | 29 ++++++++++++++++-------------
3 files changed, 21 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..e754b7c69c 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -458,7 +458,8 @@ 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] 46+ messages in thread* Re: [PATCH v4 1/7] plugins: add flag to specify whether PC is rw
2026-02-24 15:48 ` [PATCH v4 1/7] plugins: add flag to specify whether PC is rw Florian Hofhammer
@ 2026-02-24 17:41 ` Alex Bennée
0 siblings, 0 replies; 46+ messages in thread
From: Alex Bennée @ 2026-02-24 17:41 UTC (permalink / raw)
To: Florian Hofhammer
Cc: qemu-devel, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
Florian Hofhammer <florian.hofhammer@epfl.ch> writes:
> 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.
>
> 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] 46+ messages in thread
* [PATCH v4 2/7] linux-user: make syscall emulation interruptible
2026-02-24 15:46 [PATCH v4 0/7] Enable PC diversion via the plugin API Florian Hofhammer
2026-02-24 15:48 ` [PATCH v4 1/7] plugins: add flag to specify whether PC is rw Florian Hofhammer
@ 2026-02-24 15:50 ` Florian Hofhammer
2026-02-24 21:05 ` Pierrick Bouvier
2026-02-24 15:51 ` [PATCH v4 3/7] plugins: add PC diversion API function Florian Hofhammer
` (5 subsequent siblings)
7 siblings, 1 reply; 46+ messages in thread
From: Florian Hofhammer @ 2026-02-24 15:50 UTC (permalink / raw)
To: qemu-devel
Cc: alex.bennee, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
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.
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 | 4 ++++
linux-user/i386/cpu_loop.c | 8 +++++---
linux-user/include/special-errno.h | 8 ++++++++
linux-user/loongarch64/cpu_loop.c | 5 +++--
linux-user/m68k/cpu_loop.c | 2 +-
linux-user/microblaze/cpu_loop.c | 2 +-
linux-user/mips/cpu_loop.c | 5 +++--
linux-user/or1k/cpu_loop.c | 2 +-
linux-user/ppc/cpu_loop.c | 6 ++++--
linux-user/riscv/cpu_loop.c | 2 +-
linux-user/s390x/cpu_loop.c | 2 +-
linux-user/sh4/cpu_loop.c | 2 +-
linux-user/sparc/cpu_loop.c | 4 +++-
linux-user/syscall.c | 16 ++++++++++++++++
linux-user/xtensa/cpu_loop.c | 3 +++
19 files changed, 59 insertions(+), 20 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..bd3b67059b 100644
--- a/linux-user/hppa/cpu_loop.c
+++ b/linux-user/hppa/cpu_loop.c
@@ -17,6 +17,7 @@
* along with this program; if not, see <http://www.gnu.org/licenses/>.
*/
+#include "qemu/compiler.h"
#include "qemu/osdep.h"
#include "qemu.h"
#include "user-internals.h"
@@ -123,7 +124,10 @@ void cpu_loop(CPUHPPAState *env)
env->iaoq_b = env->iaoq_f + 4;
break;
case -QEMU_ERESTARTSYS:
+ QEMU_FALLTHROUGH;
case -QEMU_ESIGRETURN:
+ QEMU_FALLTHROUGH;
+ case -QEMU_ESETPC:
break;
}
break;
diff --git a/linux-user/i386/cpu_loop.c b/linux-user/i386/cpu_loop.c
index f3f58576af..fe922fceb5 100644
--- a/linux-user/i386/cpu_loop.c
+++ b/linux-user/i386/cpu_loop.c
@@ -181,7 +181,9 @@ static void emulate_vsyscall(CPUX86State *env)
if (ret == -TARGET_EFAULT) {
goto sigsegv;
}
- env->regs[R_EAX] = ret;
+ if (ret != -QEMU_ESETPC) {
+ env->regs[R_EAX] = ret;
+ }
/* Emulate a ret instruction to leave the vsyscall page. */
env->eip = caller;
@@ -234,7 +236,7 @@ void cpu_loop(CPUX86State *env)
0, 0);
if (ret == -QEMU_ERESTARTSYS) {
env->eip -= 2;
- } else if (ret != -QEMU_ESIGRETURN) {
+ } else if (ret != -QEMU_ESIGRETURN && ret != -QEMU_ESETPC) {
env->regs[R_EAX] = ret;
}
break;
@@ -253,7 +255,7 @@ void cpu_loop(CPUX86State *env)
0, 0);
if (ret == -QEMU_ERESTARTSYS) {
env->eip -= 2;
- } else if (ret != -QEMU_ESIGRETURN) {
+ } else if (ret != -QEMU_ESIGRETURN && ret != -QEMU_ESETPC) {
env->regs[R_EAX] = ret;
}
break;
diff --git a/linux-user/include/special-errno.h b/linux-user/include/special-errno.h
index 4120455baa..1db757241a 100644
--- a/linux-user/include/special-errno.h
+++ b/linux-user/include/special-errno.h
@@ -29,4 +29,12 @@
*/
#define QEMU_ESIGRETURN 513
+/*
+ * This is returned after a plugin has used the qemu_plugin_set_pc API, to
+ * indicate that the plugin deliberately changed the PC and potentially
+ * modified the register values. The main loop should not touch the guest
+ * registers for this reason.
+ */
+#define QEMU_ESETPC 514
+
#endif /* SPECIAL_ERRNO_H */
diff --git a/linux-user/loongarch64/cpu_loop.c b/linux-user/loongarch64/cpu_loop.c
index 26a5ce3a93..603fcc39c7 100644
--- a/linux-user/loongarch64/cpu_loop.c
+++ b/linux-user/loongarch64/cpu_loop.c
@@ -44,9 +44,10 @@ void cpu_loop(CPULoongArchState *env)
env->pc -= 4;
break;
}
- if (ret == -QEMU_ESIGRETURN) {
+ if (ret == -QEMU_ESIGRETURN || ret == -QEMU_ESETPC) {
/*
- * Returning from a successful sigreturn syscall.
+ * Returning from a successful sigreturn syscall or from
+ * control flow diversion in a plugin callback.
* Avoid clobbering register state.
*/
break;
diff --git a/linux-user/m68k/cpu_loop.c b/linux-user/m68k/cpu_loop.c
index 2c9f628241..b98ca8ff7b 100644
--- a/linux-user/m68k/cpu_loop.c
+++ b/linux-user/m68k/cpu_loop.c
@@ -66,7 +66,7 @@ void cpu_loop(CPUM68KState *env)
0, 0);
if (ret == -QEMU_ERESTARTSYS) {
env->pc -= 2;
- } else if (ret != -QEMU_ESIGRETURN) {
+ } else if (ret != -QEMU_ESIGRETURN && ret != -QEMU_ESETPC) {
env->dregs[0] = ret;
}
}
diff --git a/linux-user/microblaze/cpu_loop.c b/linux-user/microblaze/cpu_loop.c
index 78506ab23d..06d92c0b90 100644
--- a/linux-user/microblaze/cpu_loop.c
+++ b/linux-user/microblaze/cpu_loop.c
@@ -54,7 +54,7 @@ void cpu_loop(CPUMBState *env)
if (ret == -QEMU_ERESTARTSYS) {
/* Wind back to before the syscall. */
env->pc -= 4;
- } else if (ret != -QEMU_ESIGRETURN) {
+ } else if (ret != -QEMU_ESIGRETURN && ret != -QEMU_ESETPC) {
env->regs[3] = ret;
}
/* All syscall exits result in guest r14 being equal to the
diff --git a/linux-user/mips/cpu_loop.c b/linux-user/mips/cpu_loop.c
index 2365de1de1..af98138eb2 100644
--- a/linux-user/mips/cpu_loop.c
+++ b/linux-user/mips/cpu_loop.c
@@ -140,8 +140,9 @@ done_syscall:
env->active_tc.PC -= 4;
break;
}
- if (ret == -QEMU_ESIGRETURN) {
- /* Returning from a successful sigreturn syscall.
+ if (ret == -QEMU_ESIGRETURN || ret == -QEMU_ESETPC) {
+ /* Returning from a successful sigreturn syscall or from
+ control flow diversion in a plugin callback.
Avoid clobbering register state. */
break;
}
diff --git a/linux-user/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..1f8aae14bb 100644
--- a/linux-user/ppc/cpu_loop.c
+++ b/linux-user/ppc/cpu_loop.c
@@ -340,8 +340,10 @@ void cpu_loop(CPUPPCState *env)
env->nip -= 4;
break;
}
- if (ret == (target_ulong)(-QEMU_ESIGRETURN)) {
- /* Returning from a successful sigreturn syscall.
+ if (ret == (target_ulong)(-QEMU_ESIGRETURN)
+ || ret == (target_ulong)(-QEMU_ESETPC)) {
+ /* Returning from a successful sigreturn syscall or from
+ control flow diversion in a plugin callback.
Avoid corrupting register state. */
break;
}
diff --git a/linux-user/riscv/cpu_loop.c b/linux-user/riscv/cpu_loop.c
index ce542540c2..eecc8d1517 100644
--- a/linux-user/riscv/cpu_loop.c
+++ b/linux-user/riscv/cpu_loop.c
@@ -65,7 +65,7 @@ void cpu_loop(CPURISCVState *env)
}
if (ret == -QEMU_ERESTARTSYS) {
env->pc -= 4;
- } else if (ret != -QEMU_ESIGRETURN) {
+ } else if (ret != -QEMU_ESIGRETURN && ret != -QEMU_ESETPC) {
env->gpr[xA0] = ret;
}
if (cs->singlestep_enabled) {
diff --git a/linux-user/s390x/cpu_loop.c b/linux-user/s390x/cpu_loop.c
index 4929b32e1f..67d2a803fb 100644
--- a/linux-user/s390x/cpu_loop.c
+++ b/linux-user/s390x/cpu_loop.c
@@ -83,7 +83,7 @@ void cpu_loop(CPUS390XState *env)
env->regs[6], env->regs[7], 0, 0);
if (ret == -QEMU_ERESTARTSYS) {
env->psw.addr -= env->int_svc_ilen;
- } else if (ret != -QEMU_ESIGRETURN) {
+ } else if (ret != -QEMU_ESIGRETURN && ret != -QEMU_ESETPC) {
env->regs[2] = ret;
}
diff --git a/linux-user/sh4/cpu_loop.c b/linux-user/sh4/cpu_loop.c
index 0c9d7e9c46..ee2958d0d9 100644
--- a/linux-user/sh4/cpu_loop.c
+++ b/linux-user/sh4/cpu_loop.c
@@ -50,7 +50,7 @@ void cpu_loop(CPUSH4State *env)
0, 0);
if (ret == -QEMU_ERESTARTSYS) {
env->pc -= 2;
- } else if (ret != -QEMU_ESIGRETURN) {
+ } else if (ret != -QEMU_ESIGRETURN && ret != -QEMU_ESETPC) {
env->gregs[0] = ret;
}
break;
diff --git a/linux-user/sparc/cpu_loop.c b/linux-user/sparc/cpu_loop.c
index 7391e2add8..f054316dce 100644
--- a/linux-user/sparc/cpu_loop.c
+++ b/linux-user/sparc/cpu_loop.c
@@ -229,7 +229,9 @@ void cpu_loop (CPUSPARCState *env)
env->regwptr[2], env->regwptr[3],
env->regwptr[4], env->regwptr[5],
0, 0);
- if (ret == -QEMU_ERESTARTSYS || ret == -QEMU_ESIGRETURN) {
+ if (ret == -QEMU_ERESTARTSYS
+ || ret == -QEMU_ESIGRETURN
+ || ret == -QEMU_ESETPC) {
break;
}
if ((abi_ulong)ret >= (abi_ulong)(-515)) {
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 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..7680e243bb 100644
--- a/linux-user/xtensa/cpu_loop.c
+++ b/linux-user/xtensa/cpu_loop.c
@@ -17,6 +17,7 @@
* along with this program; if not, see <http://www.gnu.org/licenses/>.
*/
+#include "qemu/compiler.h"
#include "qemu/osdep.h"
#include "qemu.h"
#include "user-internals.h"
@@ -185,6 +186,8 @@ void cpu_loop(CPUXtensaState *env)
env->pc -= 3;
break;
+ case -QEMU_ESETPC:
+ QEMU_FALLTHROUGH;
case -QEMU_ESIGRETURN:
break;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v4 2/7] linux-user: make syscall emulation interruptible
2026-02-24 15:50 ` [PATCH v4 2/7] linux-user: make syscall emulation interruptible Florian Hofhammer
@ 2026-02-24 21:05 ` Pierrick Bouvier
2026-02-25 8:02 ` Florian Hofhammer
2026-02-25 9:25 ` Alex Bennée
0 siblings, 2 replies; 46+ messages in thread
From: Pierrick Bouvier @ 2026-02-24 21:05 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: alex.bennee, richard.henderson, laurent, imp, berrange
On 2/24/26 7:50 AM, Florian Hofhammer wrote:
> 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.
>
> 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 | 4 ++++
> linux-user/i386/cpu_loop.c | 8 +++++---
> linux-user/include/special-errno.h | 8 ++++++++
> linux-user/loongarch64/cpu_loop.c | 5 +++--
> linux-user/m68k/cpu_loop.c | 2 +-
> linux-user/microblaze/cpu_loop.c | 2 +-
> linux-user/mips/cpu_loop.c | 5 +++--
> linux-user/or1k/cpu_loop.c | 2 +-
> linux-user/ppc/cpu_loop.c | 6 ++++--
> linux-user/riscv/cpu_loop.c | 2 +-
> linux-user/s390x/cpu_loop.c | 2 +-
> linux-user/sh4/cpu_loop.c | 2 +-
> linux-user/sparc/cpu_loop.c | 4 +++-
> linux-user/syscall.c | 16 ++++++++++++++++
> linux-user/xtensa/cpu_loop.c | 3 +++
> 19 files changed, 59 insertions(+), 20 deletions(-)
>
> diff --git a/linux-user/sparc/cpu_loop.c b/linux-user/sparc/cpu_loop.c
> index 7391e2add8..f054316dce 100644
> --- a/linux-user/sparc/cpu_loop.c
> +++ b/linux-user/sparc/cpu_loop.c
> @@ -229,7 +229,9 @@ void cpu_loop (CPUSPARCState *env)
> env->regwptr[2], env->regwptr[3],
> env->regwptr[4], env->regwptr[5],
> 0, 0);
> - if (ret == -QEMU_ERESTARTSYS || ret == -QEMU_ESIGRETURN) {
> + if (ret == -QEMU_ERESTARTSYS
> + || ret == -QEMU_ESIGRETURN
> + || ret == -QEMU_ESETPC) {
> break;
> }
Just a style nit:
if (ret == -QEMU_ERESTARTSYS ||
ret == -QEMU_ESIGRETURN ||
ret == -QEMU_ESETPC) {
> 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;
> + }
> +
Makes sense, and I guess you found that when running the series test.
Do you need all the new QEMU_FALLTHROUGH introduced here? Is it for
removing warnings?
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 2/7] linux-user: make syscall emulation interruptible
2026-02-24 21:05 ` Pierrick Bouvier
@ 2026-02-25 8:02 ` Florian Hofhammer
2026-02-25 17:00 ` Pierrick Bouvier
2026-02-25 9:25 ` Alex Bennée
1 sibling, 1 reply; 46+ messages in thread
From: Florian Hofhammer @ 2026-02-25 8:02 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel
Cc: alex.bennee, richard.henderson, laurent, imp, berrange
[-- Attachment #1: Type: text/plain, Size: 4395 bytes --]
On 24/02/2026 22:05, Pierrick Bouvier wrote:
> On 2/24/26 7:50 AM, Florian Hofhammer wrote:
>> 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.
>>
>> 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 | 4 ++++
>> linux-user/i386/cpu_loop.c | 8 +++++---
>> linux-user/include/special-errno.h | 8 ++++++++
>> linux-user/loongarch64/cpu_loop.c | 5 +++--
>> linux-user/m68k/cpu_loop.c | 2 +-
>> linux-user/microblaze/cpu_loop.c | 2 +-
>> linux-user/mips/cpu_loop.c | 5 +++--
>> linux-user/or1k/cpu_loop.c | 2 +-
>> linux-user/ppc/cpu_loop.c | 6 ++++--
>> linux-user/riscv/cpu_loop.c | 2 +-
>> linux-user/s390x/cpu_loop.c | 2 +-
>> linux-user/sh4/cpu_loop.c | 2 +-
>> linux-user/sparc/cpu_loop.c | 4 +++-
>> linux-user/syscall.c | 16 ++++++++++++++++
>> linux-user/xtensa/cpu_loop.c | 3 +++
>> 19 files changed, 59 insertions(+), 20 deletions(-)
>>
>> diff --git a/linux-user/sparc/cpu_loop.c b/linux-user/sparc/cpu_loop.c
>> index 7391e2add8..f054316dce 100644
>> --- a/linux-user/sparc/cpu_loop.c
>> +++ b/linux-user/sparc/cpu_loop.c
>> @@ -229,7 +229,9 @@ void cpu_loop (CPUSPARCState *env)
>> env->regwptr[2], env->regwptr[3],
>> env->regwptr[4], env->regwptr[5],
>> 0, 0);
>> - if (ret == -QEMU_ERESTARTSYS || ret == -QEMU_ESIGRETURN) {
>> + if (ret == -QEMU_ERESTARTSYS
>> + || ret == -QEMU_ESIGRETURN
>> + || ret == -QEMU_ESETPC) {
>> break;
>> }
>
> Just a style nit:
> if (ret == -QEMU_ERESTARTSYS ||
> ret == -QEMU_ESIGRETURN ||
> ret == -QEMU_ESETPC) {
>
>> 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;
>> + }
>> +
>
> Makes sense, and I guess you found that when running the series test.
>
> Do you need all the new QEMU_FALLTHROUGH introduced here? Is it for removing warnings?
I think they're not strictly necessary, but if I recall correctly, I had
warnings otherwise.
I can remove them if you prefer but I think making the fallthrough
explicit encodes the intent better.
>
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4346 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 2/7] linux-user: make syscall emulation interruptible
2026-02-25 8:02 ` Florian Hofhammer
@ 2026-02-25 17:00 ` Pierrick Bouvier
0 siblings, 0 replies; 46+ messages in thread
From: Pierrick Bouvier @ 2026-02-25 17:00 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: alex.bennee, richard.henderson, laurent, imp, berrange
On 2/25/26 12:02 AM, Florian Hofhammer wrote:
> On 24/02/2026 22:05, Pierrick Bouvier wrote:
>> On 2/24/26 7:50 AM, Florian Hofhammer wrote:
>>> 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.
>>>
>>> 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 | 4 ++++
>>> linux-user/i386/cpu_loop.c | 8 +++++---
>>> linux-user/include/special-errno.h | 8 ++++++++
>>> linux-user/loongarch64/cpu_loop.c | 5 +++--
>>> linux-user/m68k/cpu_loop.c | 2 +-
>>> linux-user/microblaze/cpu_loop.c | 2 +-
>>> linux-user/mips/cpu_loop.c | 5 +++--
>>> linux-user/or1k/cpu_loop.c | 2 +-
>>> linux-user/ppc/cpu_loop.c | 6 ++++--
>>> linux-user/riscv/cpu_loop.c | 2 +-
>>> linux-user/s390x/cpu_loop.c | 2 +-
>>> linux-user/sh4/cpu_loop.c | 2 +-
>>> linux-user/sparc/cpu_loop.c | 4 +++-
>>> linux-user/syscall.c | 16 ++++++++++++++++
>>> linux-user/xtensa/cpu_loop.c | 3 +++
>>> 19 files changed, 59 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/linux-user/sparc/cpu_loop.c b/linux-user/sparc/cpu_loop.c
>>> index 7391e2add8..f054316dce 100644
>>> --- a/linux-user/sparc/cpu_loop.c
>>> +++ b/linux-user/sparc/cpu_loop.c
>>> @@ -229,7 +229,9 @@ void cpu_loop (CPUSPARCState *env)
>>> env->regwptr[2], env->regwptr[3],
>>> env->regwptr[4], env->regwptr[5],
>>> 0, 0);
>>> - if (ret == -QEMU_ERESTARTSYS || ret == -QEMU_ESIGRETURN) {
>>> + if (ret == -QEMU_ERESTARTSYS
>>> + || ret == -QEMU_ESIGRETURN
>>> + || ret == -QEMU_ESETPC) {
>>> break;
>>> }
>>
>> Just a style nit:
>> if (ret == -QEMU_ERESTARTSYS ||
>> ret == -QEMU_ESIGRETURN ||
>> ret == -QEMU_ESETPC) {
>>
>>> 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;
>>> + }
>>> +
>>
>> Makes sense, and I guess you found that when running the series test.
>>
>> Do you need all the new QEMU_FALLTHROUGH introduced here? Is it for removing warnings?
>
> I think they're not strictly necessary, but if I recall correctly, I had
> warnings otherwise.
> I can remove them if you prefer but I think making the fallthrough
> explicit encodes the intent better.
>
If that compiles warning-free without it, you can remove it. Else, leave
it as it is.
Fallthrough attribute is precisely used to silence warning compiler, and
not to document code, even though it can help with that.
Thanks,
Pierrick
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v4 2/7] linux-user: make syscall emulation interruptible
2026-02-24 21:05 ` Pierrick Bouvier
2026-02-25 8:02 ` Florian Hofhammer
@ 2026-02-25 9:25 ` Alex Bennée
2026-02-25 9:29 ` Florian Hofhammer
1 sibling, 1 reply; 46+ messages in thread
From: Alex Bennée @ 2026-02-25 9:25 UTC (permalink / raw)
To: Pierrick Bouvier
Cc: Florian Hofhammer, qemu-devel, richard.henderson, laurent, imp,
berrange
Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> On 2/24/26 7:50 AM, Florian Hofhammer wrote:
>> 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.
>> 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 | 4 ++++
>> linux-user/i386/cpu_loop.c | 8 +++++---
>> linux-user/include/special-errno.h | 8 ++++++++
>> linux-user/loongarch64/cpu_loop.c | 5 +++--
>> linux-user/m68k/cpu_loop.c | 2 +-
>> linux-user/microblaze/cpu_loop.c | 2 +-
>> linux-user/mips/cpu_loop.c | 5 +++--
>> linux-user/or1k/cpu_loop.c | 2 +-
>> linux-user/ppc/cpu_loop.c | 6 ++++--
>> linux-user/riscv/cpu_loop.c | 2 +-
>> linux-user/s390x/cpu_loop.c | 2 +-
>> linux-user/sh4/cpu_loop.c | 2 +-
>> linux-user/sparc/cpu_loop.c | 4 +++-
>> linux-user/syscall.c | 16 ++++++++++++++++
>> linux-user/xtensa/cpu_loop.c | 3 +++
>> 19 files changed, 59 insertions(+), 20 deletions(-)
>> diff --git a/linux-user/sparc/cpu_loop.c
>> b/linux-user/sparc/cpu_loop.c
>> index 7391e2add8..f054316dce 100644
>> --- a/linux-user/sparc/cpu_loop.c
>> +++ b/linux-user/sparc/cpu_loop.c
>> @@ -229,7 +229,9 @@ void cpu_loop (CPUSPARCState *env)
>> env->regwptr[2], env->regwptr[3],
>> env->regwptr[4], env->regwptr[5],
>> 0, 0);
>> - if (ret == -QEMU_ERESTARTSYS || ret == -QEMU_ESIGRETURN) {
>> + if (ret == -QEMU_ERESTARTSYS
>> + || ret == -QEMU_ESIGRETURN
>> + || ret == -QEMU_ESETPC) {
>> break;
>> }
>
> Just a style nit:
> if (ret == -QEMU_ERESTARTSYS ||
> ret == -QEMU_ESIGRETURN ||
> ret == -QEMU_ESETPC) {
I was hopping the ret test could be wrapped up into a helper but it
seems sparc and x86 have enough variation in handled ret codes to make
that difficult.
<snip>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 2/7] linux-user: make syscall emulation interruptible
2026-02-25 9:25 ` Alex Bennée
@ 2026-02-25 9:29 ` Florian Hofhammer
2026-02-25 12:25 ` Alex Bennée
0 siblings, 1 reply; 46+ messages in thread
From: Florian Hofhammer @ 2026-02-25 9:29 UTC (permalink / raw)
To: Alex Bennée, Pierrick Bouvier
Cc: qemu-devel, richard.henderson, laurent, imp, berrange
[-- Attachment #1: Type: text/plain, Size: 3223 bytes --]
On 25/02/2026 10:25, Alex Bennée wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>
>> On 2/24/26 7:50 AM, Florian Hofhammer wrote:
>>> 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.
>>> 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 | 4 ++++
>>> linux-user/i386/cpu_loop.c | 8 +++++---
>>> linux-user/include/special-errno.h | 8 ++++++++
>>> linux-user/loongarch64/cpu_loop.c | 5 +++--
>>> linux-user/m68k/cpu_loop.c | 2 +-
>>> linux-user/microblaze/cpu_loop.c | 2 +-
>>> linux-user/mips/cpu_loop.c | 5 +++--
>>> linux-user/or1k/cpu_loop.c | 2 +-
>>> linux-user/ppc/cpu_loop.c | 6 ++++--
>>> linux-user/riscv/cpu_loop.c | 2 +-
>>> linux-user/s390x/cpu_loop.c | 2 +-
>>> linux-user/sh4/cpu_loop.c | 2 +-
>>> linux-user/sparc/cpu_loop.c | 4 +++-
>>> linux-user/syscall.c | 16 ++++++++++++++++
>>> linux-user/xtensa/cpu_loop.c | 3 +++
>>> 19 files changed, 59 insertions(+), 20 deletions(-)
>>> diff --git a/linux-user/sparc/cpu_loop.c
>>> b/linux-user/sparc/cpu_loop.c
>>> index 7391e2add8..f054316dce 100644
>>> --- a/linux-user/sparc/cpu_loop.c
>>> +++ b/linux-user/sparc/cpu_loop.c
>>> @@ -229,7 +229,9 @@ void cpu_loop (CPUSPARCState *env)
>>> env->regwptr[2], env->regwptr[3],
>>> env->regwptr[4], env->regwptr[5],
>>> 0, 0);
>>> - if (ret == -QEMU_ERESTARTSYS || ret == -QEMU_ESIGRETURN) {
>>> + if (ret == -QEMU_ERESTARTSYS
>>> + || ret == -QEMU_ESIGRETURN
>>> + || ret == -QEMU_ESETPC) {
>>> break;
>>> }
>>
>> Just a style nit:
>> if (ret == -QEMU_ERESTARTSYS ||
>> ret == -QEMU_ESIGRETURN ||
>> ret == -QEMU_ESETPC) {
>
> I was hopping the ret test could be wrapped up into a helper but it
> seems sparc and x86 have enough variation in handled ret codes to make
> that difficult.
It's not just x86 and sparc, there generally seems to not be consensus
on whether return codes are checked via a single conditional check (as
is the case with sparc), dedicated conditional checks for each possible
error value, or via switch statements. I think moving that into a helper
would require a bit of refactoring across architectures.
I could check that out for a follow-up patch set if you think this makes
sense.
Best regards,
Florian
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4346 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 2/7] linux-user: make syscall emulation interruptible
2026-02-25 9:29 ` Florian Hofhammer
@ 2026-02-25 12:25 ` Alex Bennée
0 siblings, 0 replies; 46+ messages in thread
From: Alex Bennée @ 2026-02-25 12:25 UTC (permalink / raw)
To: Florian Hofhammer
Cc: Pierrick Bouvier, qemu-devel, richard.henderson, laurent, imp,
berrange
Florian Hofhammer <florian.hofhammer@epfl.ch> writes:
> On 25/02/2026 10:25, Alex Bennée wrote:
>> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>>
>>> On 2/24/26 7:50 AM, Florian Hofhammer wrote:
>>>> 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.
>>>> 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 | 4 ++++
>>>> linux-user/i386/cpu_loop.c | 8 +++++---
>>>> linux-user/include/special-errno.h | 8 ++++++++
>>>> linux-user/loongarch64/cpu_loop.c | 5 +++--
>>>> linux-user/m68k/cpu_loop.c | 2 +-
>>>> linux-user/microblaze/cpu_loop.c | 2 +-
>>>> linux-user/mips/cpu_loop.c | 5 +++--
>>>> linux-user/or1k/cpu_loop.c | 2 +-
>>>> linux-user/ppc/cpu_loop.c | 6 ++++--
>>>> linux-user/riscv/cpu_loop.c | 2 +-
>>>> linux-user/s390x/cpu_loop.c | 2 +-
>>>> linux-user/sh4/cpu_loop.c | 2 +-
>>>> linux-user/sparc/cpu_loop.c | 4 +++-
>>>> linux-user/syscall.c | 16 ++++++++++++++++
>>>> linux-user/xtensa/cpu_loop.c | 3 +++
>>>> 19 files changed, 59 insertions(+), 20 deletions(-)
>>>> diff --git a/linux-user/sparc/cpu_loop.c
>>>> b/linux-user/sparc/cpu_loop.c
>>>> index 7391e2add8..f054316dce 100644
>>>> --- a/linux-user/sparc/cpu_loop.c
>>>> +++ b/linux-user/sparc/cpu_loop.c
>>>> @@ -229,7 +229,9 @@ void cpu_loop (CPUSPARCState *env)
>>>> env->regwptr[2], env->regwptr[3],
>>>> env->regwptr[4], env->regwptr[5],
>>>> 0, 0);
>>>> - if (ret == -QEMU_ERESTARTSYS || ret == -QEMU_ESIGRETURN) {
>>>> + if (ret == -QEMU_ERESTARTSYS
>>>> + || ret == -QEMU_ESIGRETURN
>>>> + || ret == -QEMU_ESETPC) {
>>>> break;
>>>> }
>>>
>>> Just a style nit:
>>> if (ret == -QEMU_ERESTARTSYS ||
>>> ret == -QEMU_ESIGRETURN ||
>>> ret == -QEMU_ESETPC) {
>>
>> I was hopping the ret test could be wrapped up into a helper but it
>> seems sparc and x86 have enough variation in handled ret codes to make
>> that difficult.
>
> It's not just x86 and sparc, there generally seems to not be consensus
> on whether return codes are checked via a single conditional check (as
> is the case with sparc), dedicated conditional checks for each possible
> error value, or via switch statements. I think moving that into a helper
> would require a bit of refactoring across architectures.
> I could check that out for a follow-up patch set if you think this makes
> sense.
I think we can live with it for now:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>
> Best regards,
> Florian
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v4 3/7] plugins: add PC diversion API function
2026-02-24 15:46 [PATCH v4 0/7] Enable PC diversion via the plugin API Florian Hofhammer
2026-02-24 15:48 ` [PATCH v4 1/7] plugins: add flag to specify whether PC is rw Florian Hofhammer
2026-02-24 15:50 ` [PATCH v4 2/7] linux-user: make syscall emulation interruptible Florian Hofhammer
@ 2026-02-24 15:51 ` Florian Hofhammer
2026-02-24 17:46 ` Alex Bennée
2026-02-24 15:52 ` [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API Florian Hofhammer
` (4 subsequent siblings)
7 siblings, 1 reply; 46+ messages in thread
From: Florian Hofhammer @ 2026-02-24 15:51 UTC (permalink / raw)
To: qemu-devel
Cc: alex.bennee, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
This patch adds a plugin API function that allows diverting the program
counter during execution. A potential use case for this functionality is
to skip over parts of the code, e.g., by hooking into a specific
instruction and setting the PC to the next instruction in the callback.
Link: https://lists.nongnu.org/archive/html/qemu-devel/2025-08/msg00656.html
Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
---
include/plugins/qemu-plugin.h | 13 +++++++++++++
plugins/api.c | 13 +++++++++++++
2 files changed, 26 insertions(+)
diff --git a/include/plugins/qemu-plugin.h b/include/plugins/qemu-plugin.h
index a6ec8e275d..04c884e82b 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 only returns in case of
+ * errors.
+ */
+QEMU_PLUGIN_API
+void qemu_plugin_set_pc(uint64_t vaddr);
+
/**
* qemu_plugin_read_memory_vaddr() - read from memory using a virtual address
*
diff --git a/plugins/api.c b/plugins/api.c
index e754b7c69c..ca3e93a194 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"
@@ -466,6 +467,18 @@ 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);
+
+ if (qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS_PC) {
+ return;
+ }
+
+ cpu_set_pc(current_cpu, vaddr);
+ cpu_loop_exit(current_cpu);
+}
+
bool qemu_plugin_read_memory_vaddr(uint64_t addr, GByteArray *data, size_t len)
{
g_assert(current_cpu);
--
2.53.0
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v4 3/7] plugins: add PC diversion API function
2026-02-24 15:51 ` [PATCH v4 3/7] plugins: add PC diversion API function Florian Hofhammer
@ 2026-02-24 17:46 ` Alex Bennée
2026-02-24 20:12 ` Pierrick Bouvier
0 siblings, 1 reply; 46+ messages in thread
From: Alex Bennée @ 2026-02-24 17:46 UTC (permalink / raw)
To: Florian Hofhammer
Cc: qemu-devel, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
Florian Hofhammer <florian.hofhammer@epfl.ch> writes:
> This patch adds a plugin API function that allows diverting the program
> counter during execution. A potential use case for this functionality is
> to skip over parts of the code, e.g., by hooking into a specific
> instruction and setting the PC to the next instruction in the callback.
>
> Link: https://lists.nongnu.org/archive/html/qemu-devel/2025-08/msg00656.html
> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
> ---
> include/plugins/qemu-plugin.h | 13 +++++++++++++
> plugins/api.c | 13 +++++++++++++
> 2 files changed, 26 insertions(+)
>
> diff --git a/include/plugins/qemu-plugin.h b/include/plugins/qemu-plugin.h
> index a6ec8e275d..04c884e82b 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 only returns in case of
> + * errors.
> + */
> +QEMU_PLUGIN_API
> +void qemu_plugin_set_pc(uint64_t vaddr);
> +
> /**
> * qemu_plugin_read_memory_vaddr() - read from memory using a virtual address
> *
> diff --git a/plugins/api.c b/plugins/api.c
> index e754b7c69c..ca3e93a194 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"
> @@ -466,6 +467,18 @@ 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);
> +
> + if (qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS_PC) {
> + return;
> + }
I think its fine to assert() here - we do it elsewhere in the api, if
the user is holding it wrong we should exit now rather than leave the
plugin more confused in the future.
> +
> + 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);
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 3/7] plugins: add PC diversion API function
2026-02-24 17:46 ` Alex Bennée
@ 2026-02-24 20:12 ` Pierrick Bouvier
2026-02-25 7:55 ` Florian Hofhammer
0 siblings, 1 reply; 46+ messages in thread
From: Pierrick Bouvier @ 2026-02-24 20:12 UTC (permalink / raw)
To: Alex Bennée, Florian Hofhammer
Cc: qemu-devel, richard.henderson, laurent, imp, berrange
On 2/24/26 9:46 AM, Alex Bennée wrote:
> Florian Hofhammer <florian.hofhammer@epfl.ch> writes:
>
>> This patch adds a plugin API function that allows diverting the program
>> counter during execution. A potential use case for this functionality is
>> to skip over parts of the code, e.g., by hooking into a specific
>> instruction and setting the PC to the next instruction in the callback.
>>
>> Link: https://lists.nongnu.org/archive/html/qemu-devel/2025-08/msg00656.html
>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>> ---
>> include/plugins/qemu-plugin.h | 13 +++++++++++++
>> plugins/api.c | 13 +++++++++++++
>> 2 files changed, 26 insertions(+)
>>
>> diff --git a/include/plugins/qemu-plugin.h b/include/plugins/qemu-plugin.h
>> index a6ec8e275d..04c884e82b 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 only returns in case of
>> + * errors.
>> + */
>> +QEMU_PLUGIN_API
>> +void qemu_plugin_set_pc(uint64_t vaddr);
>> +
>> /**
>> * qemu_plugin_read_memory_vaddr() - read from memory using a virtual address
>> *
>> diff --git a/plugins/api.c b/plugins/api.c
>> index e754b7c69c..ca3e93a194 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"
>> @@ -466,6 +467,18 @@ 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);
>> +
>> + if (qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS_PC) {
>> + return;
>> + }
>
> I think its fine to assert() here - we do it elsewhere in the api, if
> the user is holding it wrong we should exit now rather than leave the
> plugin more confused in the future.
>
Agree.
With the assert instead of return,
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 3/7] plugins: add PC diversion API function
2026-02-24 20:12 ` Pierrick Bouvier
@ 2026-02-25 7:55 ` Florian Hofhammer
0 siblings, 0 replies; 46+ messages in thread
From: Florian Hofhammer @ 2026-02-25 7:55 UTC (permalink / raw)
To: Pierrick Bouvier, Alex Bennée
Cc: qemu-devel, richard.henderson, laurent, imp, berrange
[-- Attachment #1: Type: text/plain, Size: 3168 bytes --]
On 24/02/2026 21:12, Pierrick Bouvier wrote:
> On 2/24/26 9:46 AM, Alex Bennée wrote:
>> Florian Hofhammer <florian.hofhammer@epfl.ch> writes:
>>
>>> This patch adds a plugin API function that allows diverting the program
>>> counter during execution. A potential use case for this functionality is
>>> to skip over parts of the code, e.g., by hooking into a specific
>>> instruction and setting the PC to the next instruction in the callback.
>>>
>>> Link: https://lists.nongnu.org/archive/html/qemu-devel/2025-08/msg00656.html
>>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>>> ---
>>> include/plugins/qemu-plugin.h | 13 +++++++++++++
>>> plugins/api.c | 13 +++++++++++++
>>> 2 files changed, 26 insertions(+)
>>>
>>> diff --git a/include/plugins/qemu-plugin.h b/include/plugins/qemu-plugin.h
>>> index a6ec8e275d..04c884e82b 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 only returns in case of
>>> + * errors.
>>> + */
>>> +QEMU_PLUGIN_API
>>> +void qemu_plugin_set_pc(uint64_t vaddr);
>>> +
>>> /**
>>> * qemu_plugin_read_memory_vaddr() - read from memory using a virtual address
>>> *
>>> diff --git a/plugins/api.c b/plugins/api.c
>>> index e754b7c69c..ca3e93a194 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"
>>> @@ -466,6 +467,18 @@ 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);
>>> +
>>> + if (qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS_PC) {
>>> + return;
>>> + }
>>
>> I think its fine to assert() here - we do it elsewhere in the api, if
>> the user is holding it wrong we should exit now rather than leave the
>> plugin more confused in the future.
>>
>
> Agree.
> With the assert instead of return,
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Ack, will update the patch accordingly. Thanks for the feedback!
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4346 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API
2026-02-24 15:46 [PATCH v4 0/7] Enable PC diversion via the plugin API Florian Hofhammer
` (2 preceding siblings ...)
2026-02-24 15:51 ` [PATCH v4 3/7] plugins: add PC diversion API function Florian Hofhammer
@ 2026-02-24 15:52 ` Florian Hofhammer
2026-02-24 16:55 ` Brian Cain
` (4 more replies)
2026-02-24 15:53 ` [PATCH v4 5/7] plugins: add read-only property for registers Florian Hofhammer
` (3 subsequent siblings)
7 siblings, 5 replies; 46+ messages in thread
From: Florian Hofhammer @ 2026-02-24 15:52 UTC (permalink / raw)
To: qemu-devel
Cc: alex.bennee, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
The test executes a non-existent syscall, which the syscall plugin
intercepts and redirects to a clean exit.
Due to architecture-specific quirks, the architecture-specific Makefiles
require setting specific compiler and linker flags in some cases.
Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
---
tests/tcg/arm/Makefile.target | 6 +++++
tests/tcg/hexagon/Makefile.target | 7 +++++
tests/tcg/mips/Makefile.target | 6 ++++-
tests/tcg/mips64/Makefile.target | 15 +++++++++++
tests/tcg/mips64el/Makefile.target | 15 +++++++++++
tests/tcg/mipsel/Makefile.target | 15 +++++++++++
tests/tcg/multiarch/Makefile.target | 22 ++++++++++++++--
.../{ => plugin}/check-plugin-output.sh | 0
.../{ => plugin}/test-plugin-mem-access.c | 0
.../plugin/test-plugin-skip-syscalls.c | 26 +++++++++++++++++++
tests/tcg/plugins/syscall.c | 6 +++++
tests/tcg/sparc64/Makefile.target | 16 ++++++++++++
12 files changed, 131 insertions(+), 3 deletions(-)
create mode 100644 tests/tcg/mips64/Makefile.target
create mode 100644 tests/tcg/mips64el/Makefile.target
create mode 100644 tests/tcg/mipsel/Makefile.target
rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
create mode 100644 tests/tcg/sparc64/Makefile.target
diff --git a/tests/tcg/arm/Makefile.target b/tests/tcg/arm/Makefile.target
index 6189d7a0e2..0d8be9cd80 100644
--- a/tests/tcg/arm/Makefile.target
+++ b/tests/tcg/arm/Makefile.target
@@ -78,4 +78,10 @@ sha512-vector: sha512.c
ARM_TESTS += sha512-vector
+ifeq ($(CONFIG_PLUGIN),y)
+# Require emitting arm32 instructions, otherwise the vCPU might accidentally
+# try to execute Thumb instructions in arm32 mode after qemu_plugin_set_pc()
+test-plugin-skip-syscalls: CFLAGS+=-marm
+endif
+
TESTS += $(ARM_TESTS)
diff --git a/tests/tcg/hexagon/Makefile.target b/tests/tcg/hexagon/Makefile.target
index f86f02bb31..111dc405fa 100644
--- a/tests/tcg/hexagon/Makefile.target
+++ b/tests/tcg/hexagon/Makefile.target
@@ -126,3 +126,10 @@ v73_scalar: CFLAGS += -Wno-unused-function
hvx_histogram: hvx_histogram.c hvx_histogram_row.S
$(CC) $(CFLAGS) $(CROSS_CC_GUEST_CFLAGS) $^ -o $@ $(LDFLAGS)
+
+ifeq ($(CONFIG_PLUGIN),y)
+# hexagon uses clang/lld which does not support -Ttext-segment but GNU ld does
+# not generally support --image-base. Therefore, the multiarch Makefile uses
+# the GNU ld flag and we special-case here for hexagon.
+override LDFLAG_TEXT_BASE = -Wl,--image-base=0x40000
+endif
diff --git a/tests/tcg/mips/Makefile.target b/tests/tcg/mips/Makefile.target
index 5d17c1706e..d08138f17b 100644
--- a/tests/tcg/mips/Makefile.target
+++ b/tests/tcg/mips/Makefile.target
@@ -9,11 +9,15 @@ MIPS_SRC=$(SRC_PATH)/tests/tcg/mips
VPATH += $(MIPS_SRC)
# hello-mips is 32 bit only
-ifeq ($(findstring 64,$(TARGET_NAME)),)
MIPS_TESTS=hello-mips
TESTS += $(MIPS_TESTS)
hello-mips: CFLAGS+=-mno-abicalls -fno-PIC -fno-stack-protector -mabi=32
hello-mips: LDFLAGS+=-nostdlib
+
+ifeq ($(CONFIG_PLUGIN),y)
+# qemu-mips(el) returns ENOSYS without triggering syscall plugin callbacks
+run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
+ $(call skip-test, $<, "qemu-mips does not execute invalid syscalls")
endif
diff --git a/tests/tcg/mips64/Makefile.target b/tests/tcg/mips64/Makefile.target
new file mode 100644
index 0000000000..5386855efc
--- /dev/null
+++ b/tests/tcg/mips64/Makefile.target
@@ -0,0 +1,15 @@
+# -*- Mode: makefile -*-
+#
+# MIPS64 - included from tests/tcg/Makefile.target
+#
+
+MIPS64_SRC=$(SRC_PATH)/tests/tcg/mips64
+
+# Set search path for all sources
+VPATH += $(MIPS64_SRC)
+
+ifeq ($(CONFIG_PLUGIN),y)
+# Require no ABI calls to avoid $t9-relative .got address calculation on MIPS64
+test-plugin-skip-syscalls: CFLAGS+=-mno-abicalls -fno-pie
+test-plugin-skip-syscalls: LDFLAGS+=-no-pie
+endif
diff --git a/tests/tcg/mips64el/Makefile.target b/tests/tcg/mips64el/Makefile.target
new file mode 100644
index 0000000000..77ac8815fe
--- /dev/null
+++ b/tests/tcg/mips64el/Makefile.target
@@ -0,0 +1,15 @@
+# -*- Mode: makefile -*-
+#
+# MIPS64EL - included from tests/tcg/Makefile.target
+#
+
+MIPS64EL_SRC=$(SRC_PATH)/tests/tcg/mips64el
+
+# Set search path for all sources
+VPATH += $(MIPS64EL_SRC)
+
+ifeq ($(CONFIG_PLUGIN),y)
+# Require no ABI calls to avoid $t9-relative .got address calculation on MIPS64
+test-plugin-skip-syscalls: CFLAGS+=-mno-abicalls -fno-pie
+test-plugin-skip-syscalls: LDFLAGS+=-no-pie
+endif
diff --git a/tests/tcg/mipsel/Makefile.target b/tests/tcg/mipsel/Makefile.target
new file mode 100644
index 0000000000..bf1bdb56b3
--- /dev/null
+++ b/tests/tcg/mipsel/Makefile.target
@@ -0,0 +1,15 @@
+# -*- Mode: makefile -*-
+#
+# MIPSEL - included from tests/tcg/Makefile.target
+#
+
+MIPSEL_SRC=$(SRC_PATH)/tests/tcg/mipsel
+
+# Set search path for all sources
+VPATH += $(MIPSEL_SRC)
+
+ifeq ($(CONFIG_PLUGIN),y)
+# qemu-mips(el) returns ENOSYS without triggering syscall plugin callbacks
+run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
+ $(call skip-test, $<, "qemu-mipsel does not execute invalid syscalls")
+endif
diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
index 07d0b27bdd..2e2dcda425 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,27 @@ 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:
EXTRA_RUNS_WITH_PLUGIN += run-plugin-test-plugin-mem-access-with-libmem.so \
run-plugin-test-plugin-syscall-filter-with-libsyscall.so
-else
+
+# Test plugin control flow redirection by skipping system calls
+# (similar functionality to syscall filter but different mechanism)
+LDFLAG_TEXT_BASE = -Wl,-Ttext-segment=0x40000
+test-plugin-skip-syscalls: LDFLAGS += $(LDFLAG_TEXT_BASE)
+test-plugin-skip-syscalls: LDFLAGS += -Wl,--section-start,.redirect=0x20000
+run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
+
+EXTRA_RUNS_WITH_PLUGIN += run-plugin-test-plugin-skip-syscalls-with-libsyscall.so
+
+else # CONFIG_PLUGIN=n
+# Do not build the syscall skipping test if it's not tested with a plugin
+# because it will simply return an error and fail the test.
+MULTIARCH_TESTS := $(filter-out test-plugin-skip-syscalls, $(MULTIARCH_TESTS))
+
# 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-skip-syscalls.c b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
new file mode 100644
index 0000000000..1f5cbc3851
--- /dev/null
+++ b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
@@ -0,0 +1,26 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This test attempts to execute an invalid syscall. The syscall test plugin
+ * should intercept this.
+ */
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+void exit_success(void) __attribute__((section(".redirect"), noinline,
+ noreturn, used));
+
+void exit_success(void) {
+ _exit(EXIT_SUCCESS);
+}
+
+int main(int argc, char *argv[]) {
+ long ret = syscall(0xc0deUL);
+ if (ret != 0L) {
+ perror("");
+ }
+ /* We should never get here */
+ return EXIT_FAILURE;
+}
diff --git a/tests/tcg/plugins/syscall.c b/tests/tcg/plugins/syscall.c
index 5658f83087..b68e3cadf4 100644
--- a/tests/tcg/plugins/syscall.c
+++ b/tests/tcg/plugins/syscall.c
@@ -148,6 +148,12 @@ static void vcpu_syscall(qemu_plugin_id_t id, unsigned int vcpu_index,
fprintf(stderr, "Error reading memory from vaddr %"PRIu64"\n", a2);
}
}
+
+ if (num == 0xc0deUL) {
+ /* Special syscall to test the control flow redirection functionality. */
+ qemu_plugin_outs("Marker syscall detected, jump to clean exit\n");
+ qemu_plugin_set_pc(0x20000);
+ }
}
static void vcpu_syscall_ret(qemu_plugin_id_t id, unsigned int vcpu_idx,
diff --git a/tests/tcg/sparc64/Makefile.target b/tests/tcg/sparc64/Makefile.target
new file mode 100644
index 0000000000..516927a3fc
--- /dev/null
+++ b/tests/tcg/sparc64/Makefile.target
@@ -0,0 +1,16 @@
+# -*- Mode: makefile -*-
+#
+# Sparc64 - included from tests/tcg/Makefile.target
+#
+
+SPARC64_SRC=$(SRC_PATH)/tests/tcg/sparc64
+
+# Set search path for all sources
+VPATH += $(SPARC64_SRC)
+
+ifeq ($(CONFIG_PLUGIN),y)
+# The defined addresses for the binary are not aligned correctly for sparc64
+# but adjusting them breaks other architectures, so just skip it on sparc64.
+run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
+ $(call skip-test, $<, "qemu-sparc64 does not allow mapping at our given fixed address")
+endif
--
2.53.0
^ permalink raw reply related [flat|nested] 46+ messages in thread* Re: [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API
2026-02-24 15:52 ` [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API Florian Hofhammer
@ 2026-02-24 16:55 ` Brian Cain
2026-02-24 20:24 ` Pierrick Bouvier
` (3 subsequent siblings)
4 siblings, 0 replies; 46+ messages in thread
From: Brian Cain @ 2026-02-24 16:55 UTC (permalink / raw)
To: Florian Hofhammer
Cc: qemu-devel, alex.bennee, pierrick.bouvier, richard.henderson,
laurent, imp, berrange
On Tue, Feb 24, 2026 at 10:01 AM Florian Hofhammer
<florian.hofhammer@epfl.ch> wrote:
>
> The test executes a non-existent syscall, which the syscall plugin
> intercepts and redirects to a clean exit.
> Due to architecture-specific quirks, the architecture-specific Makefiles
> require setting specific compiler and linker flags in some cases.
>
> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
> ---
for hexagon:
Reviewed-by: Brian Cain <brian.cain@oss.qualcomm.com>
> tests/tcg/arm/Makefile.target | 6 +++++
> tests/tcg/hexagon/Makefile.target | 7 +++++
> tests/tcg/mips/Makefile.target | 6 ++++-
> tests/tcg/mips64/Makefile.target | 15 +++++++++++
> tests/tcg/mips64el/Makefile.target | 15 +++++++++++
> tests/tcg/mipsel/Makefile.target | 15 +++++++++++
> tests/tcg/multiarch/Makefile.target | 22 ++++++++++++++--
> .../{ => plugin}/check-plugin-output.sh | 0
> .../{ => plugin}/test-plugin-mem-access.c | 0
> .../plugin/test-plugin-skip-syscalls.c | 26 +++++++++++++++++++
> tests/tcg/plugins/syscall.c | 6 +++++
> tests/tcg/sparc64/Makefile.target | 16 ++++++++++++
> 12 files changed, 131 insertions(+), 3 deletions(-)
> create mode 100644 tests/tcg/mips64/Makefile.target
> create mode 100644 tests/tcg/mips64el/Makefile.target
> create mode 100644 tests/tcg/mipsel/Makefile.target
> rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
> rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
> create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
> create mode 100644 tests/tcg/sparc64/Makefile.target
>
> diff --git a/tests/tcg/arm/Makefile.target b/tests/tcg/arm/Makefile.target
> index 6189d7a0e2..0d8be9cd80 100644
> --- a/tests/tcg/arm/Makefile.target
> +++ b/tests/tcg/arm/Makefile.target
> @@ -78,4 +78,10 @@ sha512-vector: sha512.c
>
> ARM_TESTS += sha512-vector
>
> +ifeq ($(CONFIG_PLUGIN),y)
> +# Require emitting arm32 instructions, otherwise the vCPU might accidentally
> +# try to execute Thumb instructions in arm32 mode after qemu_plugin_set_pc()
> +test-plugin-skip-syscalls: CFLAGS+=-marm
> +endif
> +
> TESTS += $(ARM_TESTS)
> diff --git a/tests/tcg/hexagon/Makefile.target b/tests/tcg/hexagon/Makefile.target
> index f86f02bb31..111dc405fa 100644
> --- a/tests/tcg/hexagon/Makefile.target
> +++ b/tests/tcg/hexagon/Makefile.target
> @@ -126,3 +126,10 @@ v73_scalar: CFLAGS += -Wno-unused-function
>
> hvx_histogram: hvx_histogram.c hvx_histogram_row.S
> $(CC) $(CFLAGS) $(CROSS_CC_GUEST_CFLAGS) $^ -o $@ $(LDFLAGS)
> +
> +ifeq ($(CONFIG_PLUGIN),y)
> +# hexagon uses clang/lld which does not support -Ttext-segment but GNU ld does
> +# not generally support --image-base. Therefore, the multiarch Makefile uses
> +# the GNU ld flag and we special-case here for hexagon.
> +override LDFLAG_TEXT_BASE = -Wl,--image-base=0x40000
> +endif
> diff --git a/tests/tcg/mips/Makefile.target b/tests/tcg/mips/Makefile.target
> index 5d17c1706e..d08138f17b 100644
> --- a/tests/tcg/mips/Makefile.target
> +++ b/tests/tcg/mips/Makefile.target
> @@ -9,11 +9,15 @@ MIPS_SRC=$(SRC_PATH)/tests/tcg/mips
> VPATH += $(MIPS_SRC)
>
> # hello-mips is 32 bit only
> -ifeq ($(findstring 64,$(TARGET_NAME)),)
> MIPS_TESTS=hello-mips
>
> TESTS += $(MIPS_TESTS)
>
> hello-mips: CFLAGS+=-mno-abicalls -fno-PIC -fno-stack-protector -mabi=32
> hello-mips: LDFLAGS+=-nostdlib
> +
> +ifeq ($(CONFIG_PLUGIN),y)
> +# qemu-mips(el) returns ENOSYS without triggering syscall plugin callbacks
> +run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
> + $(call skip-test, $<, "qemu-mips does not execute invalid syscalls")
> endif
> diff --git a/tests/tcg/mips64/Makefile.target b/tests/tcg/mips64/Makefile.target
> new file mode 100644
> index 0000000000..5386855efc
> --- /dev/null
> +++ b/tests/tcg/mips64/Makefile.target
> @@ -0,0 +1,15 @@
> +# -*- Mode: makefile -*-
> +#
> +# MIPS64 - included from tests/tcg/Makefile.target
> +#
> +
> +MIPS64_SRC=$(SRC_PATH)/tests/tcg/mips64
> +
> +# Set search path for all sources
> +VPATH += $(MIPS64_SRC)
> +
> +ifeq ($(CONFIG_PLUGIN),y)
> +# Require no ABI calls to avoid $t9-relative .got address calculation on MIPS64
> +test-plugin-skip-syscalls: CFLAGS+=-mno-abicalls -fno-pie
> +test-plugin-skip-syscalls: LDFLAGS+=-no-pie
> +endif
> diff --git a/tests/tcg/mips64el/Makefile.target b/tests/tcg/mips64el/Makefile.target
> new file mode 100644
> index 0000000000..77ac8815fe
> --- /dev/null
> +++ b/tests/tcg/mips64el/Makefile.target
> @@ -0,0 +1,15 @@
> +# -*- Mode: makefile -*-
> +#
> +# MIPS64EL - included from tests/tcg/Makefile.target
> +#
> +
> +MIPS64EL_SRC=$(SRC_PATH)/tests/tcg/mips64el
> +
> +# Set search path for all sources
> +VPATH += $(MIPS64EL_SRC)
> +
> +ifeq ($(CONFIG_PLUGIN),y)
> +# Require no ABI calls to avoid $t9-relative .got address calculation on MIPS64
> +test-plugin-skip-syscalls: CFLAGS+=-mno-abicalls -fno-pie
> +test-plugin-skip-syscalls: LDFLAGS+=-no-pie
> +endif
> diff --git a/tests/tcg/mipsel/Makefile.target b/tests/tcg/mipsel/Makefile.target
> new file mode 100644
> index 0000000000..bf1bdb56b3
> --- /dev/null
> +++ b/tests/tcg/mipsel/Makefile.target
> @@ -0,0 +1,15 @@
> +# -*- Mode: makefile -*-
> +#
> +# MIPSEL - included from tests/tcg/Makefile.target
> +#
> +
> +MIPSEL_SRC=$(SRC_PATH)/tests/tcg/mipsel
> +
> +# Set search path for all sources
> +VPATH += $(MIPSEL_SRC)
> +
> +ifeq ($(CONFIG_PLUGIN),y)
> +# qemu-mips(el) returns ENOSYS without triggering syscall plugin callbacks
> +run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
> + $(call skip-test, $<, "qemu-mipsel does not execute invalid syscalls")
> +endif
> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
> index 07d0b27bdd..2e2dcda425 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,27 @@ 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:
>
> EXTRA_RUNS_WITH_PLUGIN += run-plugin-test-plugin-mem-access-with-libmem.so \
> run-plugin-test-plugin-syscall-filter-with-libsyscall.so
> -else
> +
> +# Test plugin control flow redirection by skipping system calls
> +# (similar functionality to syscall filter but different mechanism)
> +LDFLAG_TEXT_BASE = -Wl,-Ttext-segment=0x40000
> +test-plugin-skip-syscalls: LDFLAGS += $(LDFLAG_TEXT_BASE)
> +test-plugin-skip-syscalls: LDFLAGS += -Wl,--section-start,.redirect=0x20000
> +run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
> +
> +EXTRA_RUNS_WITH_PLUGIN += run-plugin-test-plugin-skip-syscalls-with-libsyscall.so
> +
> +else # CONFIG_PLUGIN=n
> +# Do not build the syscall skipping test if it's not tested with a plugin
> +# because it will simply return an error and fail the test.
> +MULTIARCH_TESTS := $(filter-out test-plugin-skip-syscalls, $(MULTIARCH_TESTS))
> +
> # 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-skip-syscalls.c b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
> new file mode 100644
> index 0000000000..1f5cbc3851
> --- /dev/null
> +++ b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
> @@ -0,0 +1,26 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This test attempts to execute an invalid syscall. The syscall test plugin
> + * should intercept this.
> + */
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +
> +void exit_success(void) __attribute__((section(".redirect"), noinline,
> + noreturn, used));
> +
> +void exit_success(void) {
> + _exit(EXIT_SUCCESS);
> +}
> +
> +int main(int argc, char *argv[]) {
> + long ret = syscall(0xc0deUL);
> + if (ret != 0L) {
> + perror("");
> + }
> + /* We should never get here */
> + return EXIT_FAILURE;
> +}
> diff --git a/tests/tcg/plugins/syscall.c b/tests/tcg/plugins/syscall.c
> index 5658f83087..b68e3cadf4 100644
> --- a/tests/tcg/plugins/syscall.c
> +++ b/tests/tcg/plugins/syscall.c
> @@ -148,6 +148,12 @@ static void vcpu_syscall(qemu_plugin_id_t id, unsigned int vcpu_index,
> fprintf(stderr, "Error reading memory from vaddr %"PRIu64"\n", a2);
> }
> }
> +
> + if (num == 0xc0deUL) {
> + /* Special syscall to test the control flow redirection functionality. */
> + qemu_plugin_outs("Marker syscall detected, jump to clean exit\n");
> + qemu_plugin_set_pc(0x20000);
> + }
> }
>
> static void vcpu_syscall_ret(qemu_plugin_id_t id, unsigned int vcpu_idx,
> diff --git a/tests/tcg/sparc64/Makefile.target b/tests/tcg/sparc64/Makefile.target
> new file mode 100644
> index 0000000000..516927a3fc
> --- /dev/null
> +++ b/tests/tcg/sparc64/Makefile.target
> @@ -0,0 +1,16 @@
> +# -*- Mode: makefile -*-
> +#
> +# Sparc64 - included from tests/tcg/Makefile.target
> +#
> +
> +SPARC64_SRC=$(SRC_PATH)/tests/tcg/sparc64
> +
> +# Set search path for all sources
> +VPATH += $(SPARC64_SRC)
> +
> +ifeq ($(CONFIG_PLUGIN),y)
> +# The defined addresses for the binary are not aligned correctly for sparc64
> +# but adjusting them breaks other architectures, so just skip it on sparc64.
> +run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
> + $(call skip-test, $<, "qemu-sparc64 does not allow mapping at our given fixed address")
> +endif
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API
2026-02-24 15:52 ` [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API Florian Hofhammer
2026-02-24 16:55 ` Brian Cain
@ 2026-02-24 20:24 ` Pierrick Bouvier
2026-02-25 14:58 ` Florian Hofhammer
2026-02-24 20:35 ` Pierrick Bouvier
` (2 subsequent siblings)
4 siblings, 1 reply; 46+ messages in thread
From: Pierrick Bouvier @ 2026-02-24 20:24 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: alex.bennee, richard.henderson, laurent, imp, berrange
On 2/24/26 7:52 AM, Florian Hofhammer wrote:
> The test executes a non-existent syscall, which the syscall plugin
> intercepts and redirects to a clean exit.
> Due to architecture-specific quirks, the architecture-specific Makefiles
> require setting specific compiler and linker flags in some cases.
>
> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
> ---
> tests/tcg/arm/Makefile.target | 6 +++++
> tests/tcg/hexagon/Makefile.target | 7 +++++
> tests/tcg/mips/Makefile.target | 6 ++++-
> tests/tcg/mips64/Makefile.target | 15 +++++++++++
> tests/tcg/mips64el/Makefile.target | 15 +++++++++++
> tests/tcg/mipsel/Makefile.target | 15 +++++++++++
> tests/tcg/multiarch/Makefile.target | 22 ++++++++++++++--
> .../{ => plugin}/check-plugin-output.sh | 0
> .../{ => plugin}/test-plugin-mem-access.c | 0
> .../plugin/test-plugin-skip-syscalls.c | 26 +++++++++++++++++++
> tests/tcg/plugins/syscall.c | 6 +++++
> tests/tcg/sparc64/Makefile.target | 16 ++++++++++++
> 12 files changed, 131 insertions(+), 3 deletions(-)
> create mode 100644 tests/tcg/mips64/Makefile.target
> create mode 100644 tests/tcg/mips64el/Makefile.target
> create mode 100644 tests/tcg/mipsel/Makefile.target
> rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
> rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
> create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
> create mode 100644 tests/tcg/sparc64/Makefile.target
>
> diff --git a/tests/tcg/mips/Makefile.target b/tests/tcg/mips/Makefile.target
> index 5d17c1706e..d08138f17b 100644
> --- a/tests/tcg/mips/Makefile.target
> +++ b/tests/tcg/mips/Makefile.target
> @@ -9,11 +9,15 @@ MIPS_SRC=$(SRC_PATH)/tests/tcg/mips
> VPATH += $(MIPS_SRC)
>
> # hello-mips is 32 bit only
> -ifeq ($(findstring 64,$(TARGET_NAME)),)
> MIPS_TESTS=hello-mips
>
> TESTS += $(MIPS_TESTS)
>
> hello-mips: CFLAGS+=-mno-abicalls -fno-PIC -fno-stack-protector -mabi=32
> hello-mips: LDFLAGS+=-nostdlib
> +
> +ifeq ($(CONFIG_PLUGIN),y)
> +# qemu-mips(el) returns ENOSYS without triggering syscall plugin callbacks
> +run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
> + $(call skip-test, $<, "qemu-mips does not execute invalid syscalls")
I ran into this while pulling syscall filter API recently, and found
4096 as syscall number, which seems to make all architectures happy.
See 948ffdd79b78702239aace2d32d4f581913299b3 for more details.
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API
2026-02-24 20:24 ` Pierrick Bouvier
@ 2026-02-25 14:58 ` Florian Hofhammer
2026-02-25 17:04 ` Pierrick Bouvier
0 siblings, 1 reply; 46+ messages in thread
From: Florian Hofhammer @ 2026-02-25 14:58 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel
Cc: alex.bennee, richard.henderson, laurent, imp, berrange
On 24/02/2026 21:24, Pierrick Bouvier wrote:
> On 2/24/26 7:52 AM, Florian Hofhammer wrote:
>> The test executes a non-existent syscall, which the syscall plugin
>> intercepts and redirects to a clean exit.
>> Due to architecture-specific quirks, the architecture-specific Makefiles
>> require setting specific compiler and linker flags in some cases.
>>
>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>> ---
>> tests/tcg/arm/Makefile.target | 6 +++++
>> tests/tcg/hexagon/Makefile.target | 7 +++++
>> tests/tcg/mips/Makefile.target | 6 ++++-
>> tests/tcg/mips64/Makefile.target | 15 +++++++++++
>> tests/tcg/mips64el/Makefile.target | 15 +++++++++++
>> tests/tcg/mipsel/Makefile.target | 15 +++++++++++
>> tests/tcg/multiarch/Makefile.target | 22 ++++++++++++++--
>> .../{ => plugin}/check-plugin-output.sh | 0
>> .../{ => plugin}/test-plugin-mem-access.c | 0
>> .../plugin/test-plugin-skip-syscalls.c | 26 +++++++++++++++++++
>> tests/tcg/plugins/syscall.c | 6 +++++
>> tests/tcg/sparc64/Makefile.target | 16 ++++++++++++
>> 12 files changed, 131 insertions(+), 3 deletions(-)
>> create mode 100644 tests/tcg/mips64/Makefile.target
>> create mode 100644 tests/tcg/mips64el/Makefile.target
>> create mode 100644 tests/tcg/mipsel/Makefile.target
>> rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
>> rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
>> create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>> create mode 100644 tests/tcg/sparc64/Makefile.target
>>
>> diff --git a/tests/tcg/mips/Makefile.target b/tests/tcg/mips/Makefile.target
>> index 5d17c1706e..d08138f17b 100644
>> --- a/tests/tcg/mips/Makefile.target
>> +++ b/tests/tcg/mips/Makefile.target
>> @@ -9,11 +9,15 @@ MIPS_SRC=$(SRC_PATH)/tests/tcg/mips
>> VPATH += $(MIPS_SRC)
>> # hello-mips is 32 bit only
>> -ifeq ($(findstring 64,$(TARGET_NAME)),)
>> MIPS_TESTS=hello-mips
>> TESTS += $(MIPS_TESTS)
>> hello-mips: CFLAGS+=-mno-abicalls -fno-PIC -fno-stack-protector -mabi=32
>> hello-mips: LDFLAGS+=-nostdlib
>> +
>> +ifeq ($(CONFIG_PLUGIN),y)
>> +# qemu-mips(el) returns ENOSYS without triggering syscall plugin callbacks
>> +run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
>> + $(call skip-test, $<, "qemu-mips does not execute invalid syscalls")
>
> I ran into this while pulling syscall filter API recently, and found 4096 as syscall number, which seems to make all architectures happy.
>
> See 948ffdd79b78702239aace2d32d4f581913299b3 for more details.
Please correct me if I'm wrong, but is this really sane? The comment in
the referenced commit says that 4096 isn't used in any ISA, but on the
MIPS O32 ABI, syscall numbers start at 4000. 4096 therefore corresponds
to getpriority, which would be shadowed by using this syscall number.
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API
2026-02-25 14:58 ` Florian Hofhammer
@ 2026-02-25 17:04 ` Pierrick Bouvier
2026-02-26 8:08 ` Florian Hofhammer
0 siblings, 1 reply; 46+ messages in thread
From: Pierrick Bouvier @ 2026-02-25 17:04 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: alex.bennee, richard.henderson, laurent, imp, berrange
On 2/25/26 6:58 AM, Florian Hofhammer wrote:
> On 24/02/2026 21:24, Pierrick Bouvier wrote:
>> On 2/24/26 7:52 AM, Florian Hofhammer wrote:
>>> The test executes a non-existent syscall, which the syscall plugin
>>> intercepts and redirects to a clean exit.
>>> Due to architecture-specific quirks, the architecture-specific Makefiles
>>> require setting specific compiler and linker flags in some cases.
>>>
>>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>>> ---
>>> tests/tcg/arm/Makefile.target | 6 +++++
>>> tests/tcg/hexagon/Makefile.target | 7 +++++
>>> tests/tcg/mips/Makefile.target | 6 ++++-
>>> tests/tcg/mips64/Makefile.target | 15 +++++++++++
>>> tests/tcg/mips64el/Makefile.target | 15 +++++++++++
>>> tests/tcg/mipsel/Makefile.target | 15 +++++++++++
>>> tests/tcg/multiarch/Makefile.target | 22 ++++++++++++++--
>>> .../{ => plugin}/check-plugin-output.sh | 0
>>> .../{ => plugin}/test-plugin-mem-access.c | 0
>>> .../plugin/test-plugin-skip-syscalls.c | 26 +++++++++++++++++++
>>> tests/tcg/plugins/syscall.c | 6 +++++
>>> tests/tcg/sparc64/Makefile.target | 16 ++++++++++++
>>> 12 files changed, 131 insertions(+), 3 deletions(-)
>>> create mode 100644 tests/tcg/mips64/Makefile.target
>>> create mode 100644 tests/tcg/mips64el/Makefile.target
>>> create mode 100644 tests/tcg/mipsel/Makefile.target
>>> rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
>>> rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
>>> create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>>> create mode 100644 tests/tcg/sparc64/Makefile.target
>>>
>>> diff --git a/tests/tcg/mips/Makefile.target b/tests/tcg/mips/Makefile.target
>>> index 5d17c1706e..d08138f17b 100644
>>> --- a/tests/tcg/mips/Makefile.target
>>> +++ b/tests/tcg/mips/Makefile.target
>>> @@ -9,11 +9,15 @@ MIPS_SRC=$(SRC_PATH)/tests/tcg/mips
>>> VPATH += $(MIPS_SRC)
>>> # hello-mips is 32 bit only
>>> -ifeq ($(findstring 64,$(TARGET_NAME)),)
>>> MIPS_TESTS=hello-mips
>>> TESTS += $(MIPS_TESTS)
>>> hello-mips: CFLAGS+=-mno-abicalls -fno-PIC -fno-stack-protector -mabi=32
>>> hello-mips: LDFLAGS+=-nostdlib
>>> +
>>> +ifeq ($(CONFIG_PLUGIN),y)
>>> +# qemu-mips(el) returns ENOSYS without triggering syscall plugin callbacks
>>> +run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
>>> + $(call skip-test, $<, "qemu-mips does not execute invalid syscalls")
>>
>> I ran into this while pulling syscall filter API recently, and found 4096 as syscall number, which seems to make all architectures happy.
>>
>> See 948ffdd79b78702239aace2d32d4f581913299b3 for more details.
>
> Please correct me if I'm wrong, but is this really sane? The comment in
> the referenced commit says that 4096 isn't used in any ISA, but on the
> MIPS O32 ABI, syscall numbers start at 4000. 4096 therefore corresponds
> to getpriority, which would be shadowed by using this syscall number.
I would definitely not recommend using this value in a general purpose
plugin to implement "hypercalls", it would not be sane.
That said, for the current limited test context where we now this
syscall is not called, I would say it's ok. And if it should get broken
in the future, we would catch it and could find another magic number.
Of course, if you have a better idea of syscall number, I would be very
happy to follow another rationale, as mine was mostly try and error. :)
Regards,
Pierrick
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API
2026-02-25 17:04 ` Pierrick Bouvier
@ 2026-02-26 8:08 ` Florian Hofhammer
0 siblings, 0 replies; 46+ messages in thread
From: Florian Hofhammer @ 2026-02-26 8:08 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel
Cc: alex.bennee, richard.henderson, laurent, imp, berrange
[-- Attachment #1: Type: text/plain, Size: 3929 bytes --]
On 25/02/2026 18:04, Pierrick Bouvier wrote:
> On 2/25/26 6:58 AM, Florian Hofhammer wrote:
>> On 24/02/2026 21:24, Pierrick Bouvier wrote:
>>> On 2/24/26 7:52 AM, Florian Hofhammer wrote:
>>>> The test executes a non-existent syscall, which the syscall plugin
>>>> intercepts and redirects to a clean exit.
>>>> Due to architecture-specific quirks, the architecture-specific Makefiles
>>>> require setting specific compiler and linker flags in some cases.
>>>>
>>>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>>>> ---
>>>> tests/tcg/arm/Makefile.target | 6 +++++
>>>> tests/tcg/hexagon/Makefile.target | 7 +++++
>>>> tests/tcg/mips/Makefile.target | 6 ++++-
>>>> tests/tcg/mips64/Makefile.target | 15 +++++++++++
>>>> tests/tcg/mips64el/Makefile.target | 15 +++++++++++
>>>> tests/tcg/mipsel/Makefile.target | 15 +++++++++++
>>>> tests/tcg/multiarch/Makefile.target | 22 ++++++++++++++--
>>>> .../{ => plugin}/check-plugin-output.sh | 0
>>>> .../{ => plugin}/test-plugin-mem-access.c | 0
>>>> .../plugin/test-plugin-skip-syscalls.c | 26 +++++++++++++++++++
>>>> tests/tcg/plugins/syscall.c | 6 +++++
>>>> tests/tcg/sparc64/Makefile.target | 16 ++++++++++++
>>>> 12 files changed, 131 insertions(+), 3 deletions(-)
>>>> create mode 100644 tests/tcg/mips64/Makefile.target
>>>> create mode 100644 tests/tcg/mips64el/Makefile.target
>>>> create mode 100644 tests/tcg/mipsel/Makefile.target
>>>> rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
>>>> rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
>>>> create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>>>> create mode 100644 tests/tcg/sparc64/Makefile.target
>>>>
>>>> diff --git a/tests/tcg/mips/Makefile.target b/tests/tcg/mips/Makefile.target
>>>> index 5d17c1706e..d08138f17b 100644
>>>> --- a/tests/tcg/mips/Makefile.target
>>>> +++ b/tests/tcg/mips/Makefile.target
>>>> @@ -9,11 +9,15 @@ MIPS_SRC=$(SRC_PATH)/tests/tcg/mips
>>>> VPATH += $(MIPS_SRC)
>>>> # hello-mips is 32 bit only
>>>> -ifeq ($(findstring 64,$(TARGET_NAME)),)
>>>> MIPS_TESTS=hello-mips
>>>> TESTS += $(MIPS_TESTS)
>>>> hello-mips: CFLAGS+=-mno-abicalls -fno-PIC -fno-stack-protector -mabi=32
>>>> hello-mips: LDFLAGS+=-nostdlib
>>>> +
>>>> +ifeq ($(CONFIG_PLUGIN),y)
>>>> +# qemu-mips(el) returns ENOSYS without triggering syscall plugin callbacks
>>>> +run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
>>>> + $(call skip-test, $<, "qemu-mips does not execute invalid syscalls")
>>>
>>> I ran into this while pulling syscall filter API recently, and found 4096 as syscall number, which seems to make all architectures happy.
>>>
>>> See 948ffdd79b78702239aace2d32d4f581913299b3 for more details.
>>
>> Please correct me if I'm wrong, but is this really sane? The comment in
>> the referenced commit says that 4096 isn't used in any ISA, but on the
>> MIPS O32 ABI, syscall numbers start at 4000. 4096 therefore corresponds
>> to getpriority, which would be shadowed by using this syscall number.
>
> I would definitely not recommend using this value in a general purpose plugin to implement "hypercalls", it would not be sane.
>
> That said, for the current limited test context where we now this syscall is not called, I would say it's ok. And if it should get broken in the future, we would catch it and could find another magic number.
Fair, I suppose I was overcomplicating this unnecessarily. :)
> Of course, if you have a better idea of syscall number, I would be very happy to follow another rationale, as mine was mostly try and error. :)
>
> Regards,
> Pierrick
Best regards,
Florian
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4346 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API
2026-02-24 15:52 ` [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API Florian Hofhammer
2026-02-24 16:55 ` Brian Cain
2026-02-24 20:24 ` Pierrick Bouvier
@ 2026-02-24 20:35 ` Pierrick Bouvier
2026-02-25 7:59 ` Florian Hofhammer
2026-02-24 21:28 ` Pierrick Bouvier
2026-02-25 16:21 ` Florian Hofhammer
4 siblings, 1 reply; 46+ messages in thread
From: Pierrick Bouvier @ 2026-02-24 20:35 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: alex.bennee, richard.henderson, laurent, imp, berrange
On 2/24/26 7:52 AM, Florian Hofhammer wrote:
> The test executes a non-existent syscall, which the syscall plugin
> intercepts and redirects to a clean exit.
> Due to architecture-specific quirks, the architecture-specific Makefiles
> require setting specific compiler and linker flags in some cases.
>
> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
> ---
> tests/tcg/arm/Makefile.target | 6 +++++
> tests/tcg/hexagon/Makefile.target | 7 +++++
> tests/tcg/mips/Makefile.target | 6 ++++-
> tests/tcg/mips64/Makefile.target | 15 +++++++++++
> tests/tcg/mips64el/Makefile.target | 15 +++++++++++
> tests/tcg/mipsel/Makefile.target | 15 +++++++++++
> tests/tcg/multiarch/Makefile.target | 22 ++++++++++++++--
> .../{ => plugin}/check-plugin-output.sh | 0
> .../{ => plugin}/test-plugin-mem-access.c | 0
> .../plugin/test-plugin-skip-syscalls.c | 26 +++++++++++++++++++
> tests/tcg/plugins/syscall.c | 6 +++++
> tests/tcg/sparc64/Makefile.target | 16 ++++++++++++
> 12 files changed, 131 insertions(+), 3 deletions(-)
> create mode 100644 tests/tcg/mips64/Makefile.target
> create mode 100644 tests/tcg/mips64el/Makefile.target
> create mode 100644 tests/tcg/mipsel/Makefile.target
> rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
> rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
> create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
> create mode 100644 tests/tcg/sparc64/Makefile.target
> +++ b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
> @@ -0,0 +1,26 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This test attempts to execute an invalid syscall. The syscall test plugin
> + * should intercept this.
> + */
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +
> +void exit_success(void) __attribute__((section(".redirect"), noinline,
> + noreturn, used));
> +
> +void exit_success(void) {
> + _exit(EXIT_SUCCESS);
> +}
> +
> +int main(int argc, char *argv[]) {
> + long ret = syscall(0xc0deUL);
> + if (ret != 0L) {
> + perror("");
> + }
> + /* We should never get here */
> + return EXIT_FAILURE;
> +}
> diff --git a/tests/tcg/plugins/syscall.c b/tests/tcg/plugins/syscall.c
> index 5658f83087..b68e3cadf4 100644
> --- a/tests/tcg/plugins/syscall.c
> +++ b/tests/tcg/plugins/syscall.c
> @@ -148,6 +148,12 @@ static void vcpu_syscall(qemu_plugin_id_t id, unsigned int vcpu_index,
> fprintf(stderr, "Error reading memory from vaddr %"PRIu64"\n", a2);
> }
> }
> +
> + if (num == 0xc0deUL) {
> + /* Special syscall to test the control flow redirection functionality. */
> + qemu_plugin_outs("Marker syscall detected, jump to clean exit\n");
> + qemu_plugin_set_pc(0x20000);
An even better alternative is to use a value label, which is a gcc
extension, and you would not even need another function. Just pass it as
first parameter of syscall, and jump to this address directly from
syscall filter.
int main(int argc, char *argv[]) {
long ret = syscall(0xc0deUL, &&set_pc_dest);
/* We should never get here */
return EXIT_FAILURE;
set_pc_dest:
return EXIT_SUCCESS;
}
More details:
https://www.amulettechnologies.com/boosting-bytecode-efficiency-the-power-of-gccs-label-as-value/
https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
Regards,
Pierrick
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API
2026-02-24 20:35 ` Pierrick Bouvier
@ 2026-02-25 7:59 ` Florian Hofhammer
2026-02-25 11:49 ` Florian Hofhammer
0 siblings, 1 reply; 46+ messages in thread
From: Florian Hofhammer @ 2026-02-25 7:59 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel
Cc: alex.bennee, richard.henderson, laurent, imp, berrange
[-- Attachment #1: Type: text/plain, Size: 3964 bytes --]
On 24/02/2026 21:35, Pierrick Bouvier wrote:
> On 2/24/26 7:52 AM, Florian Hofhammer wrote:
>> The test executes a non-existent syscall, which the syscall plugin
>> intercepts and redirects to a clean exit.
>> Due to architecture-specific quirks, the architecture-specific Makefiles
>> require setting specific compiler and linker flags in some cases.
>>
>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>> ---
>> tests/tcg/arm/Makefile.target | 6 +++++
>> tests/tcg/hexagon/Makefile.target | 7 +++++
>> tests/tcg/mips/Makefile.target | 6 ++++-
>> tests/tcg/mips64/Makefile.target | 15 +++++++++++
>> tests/tcg/mips64el/Makefile.target | 15 +++++++++++
>> tests/tcg/mipsel/Makefile.target | 15 +++++++++++
>> tests/tcg/multiarch/Makefile.target | 22 ++++++++++++++--
>> .../{ => plugin}/check-plugin-output.sh | 0
>> .../{ => plugin}/test-plugin-mem-access.c | 0
>> .../plugin/test-plugin-skip-syscalls.c | 26 +++++++++++++++++++
>> tests/tcg/plugins/syscall.c | 6 +++++
>> tests/tcg/sparc64/Makefile.target | 16 ++++++++++++
>> 12 files changed, 131 insertions(+), 3 deletions(-)
>> create mode 100644 tests/tcg/mips64/Makefile.target
>> create mode 100644 tests/tcg/mips64el/Makefile.target
>> create mode 100644 tests/tcg/mipsel/Makefile.target
>> rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
>> rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
>> create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>> create mode 100644 tests/tcg/sparc64/Makefile.target
>> +++ b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>> @@ -0,0 +1,26 @@
>> +/*
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + *
>> + * This test attempts to execute an invalid syscall. The syscall test plugin
>> + * should intercept this.
>> + */
>> +#include <stdint.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <unistd.h>
>> +
>> +void exit_success(void) __attribute__((section(".redirect"), noinline,
>> + noreturn, used));
>> +
>> +void exit_success(void) {
>> + _exit(EXIT_SUCCESS);
>> +}
>> +
>> +int main(int argc, char *argv[]) {
>> + long ret = syscall(0xc0deUL);
>> + if (ret != 0L) {
>> + perror("");
>> + }
>> + /* We should never get here */
>> + return EXIT_FAILURE;
>> +}
>> diff --git a/tests/tcg/plugins/syscall.c b/tests/tcg/plugins/syscall.c
>> index 5658f83087..b68e3cadf4 100644
>> --- a/tests/tcg/plugins/syscall.c
>> +++ b/tests/tcg/plugins/syscall.c
>> @@ -148,6 +148,12 @@ static void vcpu_syscall(qemu_plugin_id_t id, unsigned int vcpu_index,
>> fprintf(stderr, "Error reading memory from vaddr %"PRIu64"\n", a2);
>> }
>> }
>> +
>> + if (num == 0xc0deUL) {
>> + /* Special syscall to test the control flow redirection functionality. */
>> + qemu_plugin_outs("Marker syscall detected, jump to clean exit\n");
>> + qemu_plugin_set_pc(0x20000);
>
> An even better alternative is to use a value label, which is a gcc extension, and you would not even need another function. Just pass it as first parameter of syscall, and jump to this address directly from syscall filter.
>
> int main(int argc, char *argv[]) {
> long ret = syscall(0xc0deUL, &&set_pc_dest);
> /* We should never get here */
> return EXIT_FAILURE;
> set_pc_dest:
> return EXIT_SUCCESS;
> }
>
> More details:
> https://www.amulettechnologies.com/boosting-bytecode-efficiency-the-power-of-gccs-label-as-value/
> https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
>
> Regards,
> Pierrick
Thanks for the idea, I didn't think about this. I'll check it out!
Best regards,
Florian
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4346 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API
2026-02-25 7:59 ` Florian Hofhammer
@ 2026-02-25 11:49 ` Florian Hofhammer
2026-02-25 17:07 ` Pierrick Bouvier
0 siblings, 1 reply; 46+ messages in thread
From: Florian Hofhammer @ 2026-02-25 11:49 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel
Cc: alex.bennee, richard.henderson, laurent, imp, berrange
[-- Attachment #1: Type: text/plain, Size: 4419 bytes --]
On 25/02/2026 08:59, Florian Hofhammer wrote:
> On 24/02/2026 21:35, Pierrick Bouvier wrote:
>> On 2/24/26 7:52 AM, Florian Hofhammer wrote:
>>> The test executes a non-existent syscall, which the syscall plugin
>>> intercepts and redirects to a clean exit.
>>> Due to architecture-specific quirks, the architecture-specific Makefiles
>>> require setting specific compiler and linker flags in some cases.
>>>
>>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>>> ---
>>> tests/tcg/arm/Makefile.target | 6 +++++
>>> tests/tcg/hexagon/Makefile.target | 7 +++++
>>> tests/tcg/mips/Makefile.target | 6 ++++-
>>> tests/tcg/mips64/Makefile.target | 15 +++++++++++
>>> tests/tcg/mips64el/Makefile.target | 15 +++++++++++
>>> tests/tcg/mipsel/Makefile.target | 15 +++++++++++
>>> tests/tcg/multiarch/Makefile.target | 22 ++++++++++++++--
>>> .../{ => plugin}/check-plugin-output.sh | 0
>>> .../{ => plugin}/test-plugin-mem-access.c | 0
>>> .../plugin/test-plugin-skip-syscalls.c | 26 +++++++++++++++++++
>>> tests/tcg/plugins/syscall.c | 6 +++++
>>> tests/tcg/sparc64/Makefile.target | 16 ++++++++++++
>>> 12 files changed, 131 insertions(+), 3 deletions(-)
>>> create mode 100644 tests/tcg/mips64/Makefile.target
>>> create mode 100644 tests/tcg/mips64el/Makefile.target
>>> create mode 100644 tests/tcg/mipsel/Makefile.target
>>> rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
>>> rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
>>> create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>>> create mode 100644 tests/tcg/sparc64/Makefile.target
>>> +++ b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>>> @@ -0,0 +1,26 @@
>>> +/*
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + *
>>> + * This test attempts to execute an invalid syscall. The syscall test plugin
>>> + * should intercept this.
>>> + */
>>> +#include <stdint.h>
>>> +#include <stdio.h>
>>> +#include <stdlib.h>
>>> +#include <unistd.h>
>>> +
>>> +void exit_success(void) __attribute__((section(".redirect"), noinline,
>>> + noreturn, used));
>>> +
>>> +void exit_success(void) {
>>> + _exit(EXIT_SUCCESS);
>>> +}
>>> +
>>> +int main(int argc, char *argv[]) {
>>> + long ret = syscall(0xc0deUL);
>>> + if (ret != 0L) {
>>> + perror("");
>>> + }
>>> + /* We should never get here */
>>> + return EXIT_FAILURE;
>>> +}
>>> diff --git a/tests/tcg/plugins/syscall.c b/tests/tcg/plugins/syscall.c
>>> index 5658f83087..b68e3cadf4 100644
>>> --- a/tests/tcg/plugins/syscall.c
>>> +++ b/tests/tcg/plugins/syscall.c
>>> @@ -148,6 +148,12 @@ static void vcpu_syscall(qemu_plugin_id_t id, unsigned int vcpu_index,
>>> fprintf(stderr, "Error reading memory from vaddr %"PRIu64"\n", a2);
>>> }
>>> }
>>> +
>>> + if (num == 0xc0deUL) {
>>> + /* Special syscall to test the control flow redirection functionality. */
>>> + qemu_plugin_outs("Marker syscall detected, jump to clean exit\n");
>>> + qemu_plugin_set_pc(0x20000);
>>
>> An even better alternative is to use a value label, which is a gcc extension, and you would not even need another function. Just pass it as first parameter of syscall, and jump to this address directly from syscall filter.
>>
>> int main(int argc, char *argv[]) {
>> long ret = syscall(0xc0deUL, &&set_pc_dest);
>> /* We should never get here */
>> return EXIT_FAILURE;
>> set_pc_dest:
>> return EXIT_SUCCESS;
>> }
>>
>> More details:
>> https://www.amulettechnologies.com/boosting-bytecode-efficiency-the-power-of-gccs-label-as-value/
>> https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
>>
>> Regards,
>> Pierrick
>
> Thanks for the idea, I didn't think about this. I'll check it out!
>
> Best regards,
> Florian
Fun finding: GCC optimizes the label and the second return away and
there's no way to turn this behavior off, so the test doesn't work with
value labels. I'm nevertheless changing to just passing the function
address as a paremeter, which is cleaner than using a hardcoded address.
Best regards,
Florian
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4346 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API
2026-02-25 11:49 ` Florian Hofhammer
@ 2026-02-25 17:07 ` Pierrick Bouvier
2026-02-25 17:09 ` Pierrick Bouvier
0 siblings, 1 reply; 46+ messages in thread
From: Pierrick Bouvier @ 2026-02-25 17:07 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: alex.bennee, richard.henderson, laurent, imp, berrange
On 2/25/26 3:49 AM, Florian Hofhammer wrote:
> On 25/02/2026 08:59, Florian Hofhammer wrote:
>> On 24/02/2026 21:35, Pierrick Bouvier wrote:
>>> On 2/24/26 7:52 AM, Florian Hofhammer wrote:
>>>> The test executes a non-existent syscall, which the syscall plugin
>>>> intercepts and redirects to a clean exit.
>>>> Due to architecture-specific quirks, the architecture-specific Makefiles
>>>> require setting specific compiler and linker flags in some cases.
>>>>
>>>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>>>> ---
>>>> tests/tcg/arm/Makefile.target | 6 +++++
>>>> tests/tcg/hexagon/Makefile.target | 7 +++++
>>>> tests/tcg/mips/Makefile.target | 6 ++++-
>>>> tests/tcg/mips64/Makefile.target | 15 +++++++++++
>>>> tests/tcg/mips64el/Makefile.target | 15 +++++++++++
>>>> tests/tcg/mipsel/Makefile.target | 15 +++++++++++
>>>> tests/tcg/multiarch/Makefile.target | 22 ++++++++++++++--
>>>> .../{ => plugin}/check-plugin-output.sh | 0
>>>> .../{ => plugin}/test-plugin-mem-access.c | 0
>>>> .../plugin/test-plugin-skip-syscalls.c | 26 +++++++++++++++++++
>>>> tests/tcg/plugins/syscall.c | 6 +++++
>>>> tests/tcg/sparc64/Makefile.target | 16 ++++++++++++
>>>> 12 files changed, 131 insertions(+), 3 deletions(-)
>>>> create mode 100644 tests/tcg/mips64/Makefile.target
>>>> create mode 100644 tests/tcg/mips64el/Makefile.target
>>>> create mode 100644 tests/tcg/mipsel/Makefile.target
>>>> rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
>>>> rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
>>>> create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>>>> create mode 100644 tests/tcg/sparc64/Makefile.target
>>>> +++ b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>>>> @@ -0,0 +1,26 @@
>>>> +/*
>>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>>> + *
>>>> + * This test attempts to execute an invalid syscall. The syscall test plugin
>>>> + * should intercept this.
>>>> + */
>>>> +#include <stdint.h>
>>>> +#include <stdio.h>
>>>> +#include <stdlib.h>
>>>> +#include <unistd.h>
>>>> +
>>>> +void exit_success(void) __attribute__((section(".redirect"), noinline,
>>>> + noreturn, used));
>>>> +
>>>> +void exit_success(void) {
>>>> + _exit(EXIT_SUCCESS);
>>>> +}
>>>> +
>>>> +int main(int argc, char *argv[]) {
>>>> + long ret = syscall(0xc0deUL);
>>>> + if (ret != 0L) {
>>>> + perror("");
>>>> + }
>>>> + /* We should never get here */
>>>> + return EXIT_FAILURE;
>>>> +}
>>>> diff --git a/tests/tcg/plugins/syscall.c b/tests/tcg/plugins/syscall.c
>>>> index 5658f83087..b68e3cadf4 100644
>>>> --- a/tests/tcg/plugins/syscall.c
>>>> +++ b/tests/tcg/plugins/syscall.c
>>>> @@ -148,6 +148,12 @@ static void vcpu_syscall(qemu_plugin_id_t id, unsigned int vcpu_index,
>>>> fprintf(stderr, "Error reading memory from vaddr %"PRIu64"\n", a2);
>>>> }
>>>> }
>>>> +
>>>> + if (num == 0xc0deUL) {
>>>> + /* Special syscall to test the control flow redirection functionality. */
>>>> + qemu_plugin_outs("Marker syscall detected, jump to clean exit\n");
>>>> + qemu_plugin_set_pc(0x20000);
>>>
>>> An even better alternative is to use a value label, which is a gcc extension, and you would not even need another function. Just pass it as first parameter of syscall, and jump to this address directly from syscall filter.
>>>
>>> int main(int argc, char *argv[]) {
>>> long ret = syscall(0xc0deUL, &&set_pc_dest);
>>> /* We should never get here */
>>> return EXIT_FAILURE;
>>> set_pc_dest:
>>> return EXIT_SUCCESS;
>>> }
>>>
>>> More details:
>>> https://www.amulettechnologies.com/boosting-bytecode-efficiency-the-power-of-gccs-label-as-value/
>>> https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
>>>
>>> Regards,
>>> Pierrick
>>
>> Thanks for the idea, I didn't think about this. I'll check it out!
>>
>> Best regards,
>> Florian
>
> Fun finding: GCC optimizes the label and the second return away and
> there's no way to turn this behavior off, so the test doesn't work with
> value labels. I'm nevertheless changing to just passing the function
> address as a paremeter, which is cleaner than using a hardcoded address.
>
Oh interesting. Is the test compiled in O0?
Yes, you can pass the function address as well. I would be happy with a
hardcoded address, but the major pain is all those flags per arch.
> Best regards,
> Florian
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API
2026-02-25 17:07 ` Pierrick Bouvier
@ 2026-02-25 17:09 ` Pierrick Bouvier
0 siblings, 0 replies; 46+ messages in thread
From: Pierrick Bouvier @ 2026-02-25 17:09 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: alex.bennee, richard.henderson, laurent, imp, berrange
On 2/25/26 9:07 AM, Pierrick Bouvier wrote:
> On 2/25/26 3:49 AM, Florian Hofhammer wrote:
>> On 25/02/2026 08:59, Florian Hofhammer wrote:
>>> On 24/02/2026 21:35, Pierrick Bouvier wrote:
>>>> On 2/24/26 7:52 AM, Florian Hofhammer wrote:
>>>>> The test executes a non-existent syscall, which the syscall plugin
>>>>> intercepts and redirects to a clean exit.
>>>>> Due to architecture-specific quirks, the architecture-specific Makefiles
>>>>> require setting specific compiler and linker flags in some cases.
>>>>>
>>>>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>>>>> ---
>>>>> tests/tcg/arm/Makefile.target | 6 +++++
>>>>> tests/tcg/hexagon/Makefile.target | 7 +++++
>>>>> tests/tcg/mips/Makefile.target | 6 ++++-
>>>>> tests/tcg/mips64/Makefile.target | 15 +++++++++++
>>>>> tests/tcg/mips64el/Makefile.target | 15 +++++++++++
>>>>> tests/tcg/mipsel/Makefile.target | 15 +++++++++++
>>>>> tests/tcg/multiarch/Makefile.target | 22 ++++++++++++++--
>>>>> .../{ => plugin}/check-plugin-output.sh | 0
>>>>> .../{ => plugin}/test-plugin-mem-access.c | 0
>>>>> .../plugin/test-plugin-skip-syscalls.c | 26 +++++++++++++++++++
>>>>> tests/tcg/plugins/syscall.c | 6 +++++
>>>>> tests/tcg/sparc64/Makefile.target | 16 ++++++++++++
>>>>> 12 files changed, 131 insertions(+), 3 deletions(-)
>>>>> create mode 100644 tests/tcg/mips64/Makefile.target
>>>>> create mode 100644 tests/tcg/mips64el/Makefile.target
>>>>> create mode 100644 tests/tcg/mipsel/Makefile.target
>>>>> rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
>>>>> rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
>>>>> create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>>>>> create mode 100644 tests/tcg/sparc64/Makefile.target
>>>>> +++ b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>>>>> @@ -0,0 +1,26 @@
>>>>> +/*
>>>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>>>> + *
>>>>> + * This test attempts to execute an invalid syscall. The syscall test plugin
>>>>> + * should intercept this.
>>>>> + */
>>>>> +#include <stdint.h>
>>>>> +#include <stdio.h>
>>>>> +#include <stdlib.h>
>>>>> +#include <unistd.h>
>>>>> +
>>>>> +void exit_success(void) __attribute__((section(".redirect"), noinline,
>>>>> + noreturn, used));
>>>>> +
>>>>> +void exit_success(void) {
>>>>> + _exit(EXIT_SUCCESS);
>>>>> +}
>>>>> +
>>>>> +int main(int argc, char *argv[]) {
>>>>> + long ret = syscall(0xc0deUL);
>>>>> + if (ret != 0L) {
>>>>> + perror("");
>>>>> + }
>>>>> + /* We should never get here */
>>>>> + return EXIT_FAILURE;
>>>>> +}
>>>>> diff --git a/tests/tcg/plugins/syscall.c b/tests/tcg/plugins/syscall.c
>>>>> index 5658f83087..b68e3cadf4 100644
>>>>> --- a/tests/tcg/plugins/syscall.c
>>>>> +++ b/tests/tcg/plugins/syscall.c
>>>>> @@ -148,6 +148,12 @@ static void vcpu_syscall(qemu_plugin_id_t id, unsigned int vcpu_index,
>>>>> fprintf(stderr, "Error reading memory from vaddr %"PRIu64"\n", a2);
>>>>> }
>>>>> }
>>>>> +
>>>>> + if (num == 0xc0deUL) {
>>>>> + /* Special syscall to test the control flow redirection functionality. */
>>>>> + qemu_plugin_outs("Marker syscall detected, jump to clean exit\n");
>>>>> + qemu_plugin_set_pc(0x20000);
>>>>
>>>> An even better alternative is to use a value label, which is a gcc extension, and you would not even need another function. Just pass it as first parameter of syscall, and jump to this address directly from syscall filter.
>>>>
>>>> int main(int argc, char *argv[]) {
>>>> long ret = syscall(0xc0deUL, &&set_pc_dest);
>>>> /* We should never get here */
>>>> return EXIT_FAILURE;
>>>> set_pc_dest:
>>>> return EXIT_SUCCESS;
>>>> }
>>>>
>>>> More details:
>>>> https://www.amulettechnologies.com/boosting-bytecode-efficiency-the-power-of-gccs-label-as-value/
>>>> https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
>>>>
>>>> Regards,
>>>> Pierrick
>>>
>>> Thanks for the idea, I didn't think about this. I'll check it out!
>>>
>>> Best regards,
>>> Florian
>>
>> Fun finding: GCC optimizes the label and the second return away and
>> there's no way to turn this behavior off, so the test doesn't work with
>> value labels. I'm nevertheless changing to just passing the function
>> address as a paremeter, which is cleaner than using a hardcoded address.
>>
>
> Oh interesting. Is the test compiled in O0?
Just saw your answer below :)
> Yes, you can pass the function address as well. I would be happy with a
> hardcoded address, but the major pain is all those flags per arch.
>
>> Best regards,
>> Florian
>
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API
2026-02-24 15:52 ` [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API Florian Hofhammer
` (2 preceding siblings ...)
2026-02-24 20:35 ` Pierrick Bouvier
@ 2026-02-24 21:28 ` Pierrick Bouvier
2026-02-25 8:03 ` Florian Hofhammer
2026-02-25 16:21 ` Florian Hofhammer
4 siblings, 1 reply; 46+ messages in thread
From: Pierrick Bouvier @ 2026-02-24 21:28 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: alex.bennee, richard.henderson, laurent, imp, berrange
On 2/24/26 7:52 AM, Florian Hofhammer wrote:
> The test executes a non-existent syscall, which the syscall plugin
> intercepts and redirects to a clean exit.
> Due to architecture-specific quirks, the architecture-specific Makefiles
> require setting specific compiler and linker flags in some cases.
>
> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
> ---
> tests/tcg/arm/Makefile.target | 6 +++++
> tests/tcg/hexagon/Makefile.target | 7 +++++
> tests/tcg/mips/Makefile.target | 6 ++++-
> tests/tcg/mips64/Makefile.target | 15 +++++++++++
> tests/tcg/mips64el/Makefile.target | 15 +++++++++++
> tests/tcg/mipsel/Makefile.target | 15 +++++++++++
> tests/tcg/multiarch/Makefile.target | 22 ++++++++++++++--
> .../{ => plugin}/check-plugin-output.sh | 0
> .../{ => plugin}/test-plugin-mem-access.c | 0
> .../plugin/test-plugin-skip-syscalls.c | 26 +++++++++++++++++++
> tests/tcg/plugins/syscall.c | 6 +++++
> tests/tcg/sparc64/Makefile.target | 16 ++++++++++++
> 12 files changed, 131 insertions(+), 3 deletions(-)
> create mode 100644 tests/tcg/mips64/Makefile.target
> create mode 100644 tests/tcg/mips64el/Makefile.target
> create mode 100644 tests/tcg/mipsel/Makefile.target
> rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
> rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
> create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
> create mode 100644 tests/tcg/sparc64/Makefile.target
>
> +++ b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
> @@ -0,0 +1,26 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This test attempts to execute an invalid syscall. The syscall test plugin
> + * should intercept this.
> + */
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +
> +void exit_success(void) __attribute__((section(".redirect"), noinline,
> + noreturn, used));
> +
> +void exit_success(void) {
> + _exit(EXIT_SUCCESS);
> +}
> +
> +int main(int argc, char *argv[]) {
> + long ret = syscall(0xc0deUL);
> + if (ret != 0L) {
> + perror("");
> + }
> + /* We should never get here */
> + return EXIT_FAILURE;
> +}
After reviewing patch 2, I think it could be nice to test also the path
redirecting pc not from a syscall, in addition to what this test already
does.
You can reuse syscall mechanic to communicate with plugin to share
address that should crash and which destination it should reach instead.
And while at it, you can add the same thing from a signal handler
(reusing syscall trick), so it should cover most of "special" cases.
You can merge all test cases in a single test.c.
Also, you can create a new set_pc plugin combining all this, because
other test cases don't really belong to tests/tcg/plugins/syscall.c.
test.c:
int main(int argc, char *argv[]) {
syscall(..., &&skip, &&dest);
error:
g_assert_not_reached();
dest:
return EXIT_SUCCESS;
}
plugin.c:
static void* error;
static void* dest;
void on_syscall(num, a1, a2) {
if (num == magic) {
error = a1;
dest = a2;
}
return true;
}
void insn_exec(..., void* pc) {
if (pc == error) {
qemu_plugin_set_pc(dest);
}
}
Regards,
Pierrick
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API
2026-02-24 21:28 ` Pierrick Bouvier
@ 2026-02-25 8:03 ` Florian Hofhammer
0 siblings, 0 replies; 46+ messages in thread
From: Florian Hofhammer @ 2026-02-25 8:03 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel
Cc: alex.bennee, richard.henderson, laurent, imp, berrange
[-- Attachment #1: Type: text/plain, Size: 3822 bytes --]
On 24/02/2026 22:28, Pierrick Bouvier wrote:
> On 2/24/26 7:52 AM, Florian Hofhammer wrote:
>> The test executes a non-existent syscall, which the syscall plugin
>> intercepts and redirects to a clean exit.
>> Due to architecture-specific quirks, the architecture-specific Makefiles
>> require setting specific compiler and linker flags in some cases.
>>
>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>> ---
>> tests/tcg/arm/Makefile.target | 6 +++++
>> tests/tcg/hexagon/Makefile.target | 7 +++++
>> tests/tcg/mips/Makefile.target | 6 ++++-
>> tests/tcg/mips64/Makefile.target | 15 +++++++++++
>> tests/tcg/mips64el/Makefile.target | 15 +++++++++++
>> tests/tcg/mipsel/Makefile.target | 15 +++++++++++
>> tests/tcg/multiarch/Makefile.target | 22 ++++++++++++++--
>> .../{ => plugin}/check-plugin-output.sh | 0
>> .../{ => plugin}/test-plugin-mem-access.c | 0
>> .../plugin/test-plugin-skip-syscalls.c | 26 +++++++++++++++++++
>> tests/tcg/plugins/syscall.c | 6 +++++
>> tests/tcg/sparc64/Makefile.target | 16 ++++++++++++
>> 12 files changed, 131 insertions(+), 3 deletions(-)
>> create mode 100644 tests/tcg/mips64/Makefile.target
>> create mode 100644 tests/tcg/mips64el/Makefile.target
>> create mode 100644 tests/tcg/mipsel/Makefile.target
>> rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
>> rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
>> create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>> create mode 100644 tests/tcg/sparc64/Makefile.target
>>
>> +++ b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>> @@ -0,0 +1,26 @@
>> +/*
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + *
>> + * This test attempts to execute an invalid syscall. The syscall test plugin
>> + * should intercept this.
>> + */
>> +#include <stdint.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <unistd.h>
>> +
>> +void exit_success(void) __attribute__((section(".redirect"), noinline,
>> + noreturn, used));
>> +
>> +void exit_success(void) {
>> + _exit(EXIT_SUCCESS);
>> +}
>> +
>> +int main(int argc, char *argv[]) {
>> + long ret = syscall(0xc0deUL);
>> + if (ret != 0L) {
>> + perror("");
>> + }
>> + /* We should never get here */
>> + return EXIT_FAILURE;
>> +}
> After reviewing patch 2, I think it could be nice to test also the path redirecting pc not from a syscall, in addition to what this test already does.
>
> You can reuse syscall mechanic to communicate with plugin to share address that should crash and which destination it should reach instead.
>
> And while at it, you can add the same thing from a signal handler (reusing syscall trick), so it should cover most of "special" cases.
>
> You can merge all test cases in a single test.c.
> Also, you can create a new set_pc plugin combining all this, because other test cases don't really belong to tests/tcg/plugins/syscall.c.
>
> test.c:
> int main(int argc, char *argv[]) {
> syscall(..., &&skip, &&dest);
> error:
> g_assert_not_reached();
> dest:
> return EXIT_SUCCESS;
> }
>
> plugin.c:
> static void* error;
> static void* dest;
>
> void on_syscall(num, a1, a2) {
> if (num == magic) {
> error = a1;
> dest = a2;
> }
> return true;
> }
>
> void insn_exec(..., void* pc) {
> if (pc == error) {
> qemu_plugin_set_pc(dest);
> }
> }
>
> Regards,
> Pierrick
Makes sense, I'll factor that out and add additional tests for the
functionality.
Best regards,
Florian
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4346 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API
2026-02-24 15:52 ` [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API Florian Hofhammer
` (3 preceding siblings ...)
2026-02-24 21:28 ` Pierrick Bouvier
@ 2026-02-25 16:21 ` Florian Hofhammer
2026-02-25 17:30 ` Pierrick Bouvier
4 siblings, 1 reply; 46+ messages in thread
From: Florian Hofhammer @ 2026-02-25 16:21 UTC (permalink / raw)
To: alex.bennee, pierrick.bouvier
Cc: richard.henderson, laurent, imp, berrange, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 13487 bytes --]
On 24/02/2026 16:52, Florian Hofhammer wrote:
> The test executes a non-existent syscall, which the syscall plugin
> intercepts and redirects to a clean exit.
> Due to architecture-specific quirks, the architecture-specific Makefiles
> require setting specific compiler and linker flags in some cases.
>
> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
> ---
> tests/tcg/arm/Makefile.target | 6 +++++
> tests/tcg/hexagon/Makefile.target | 7 +++++
> tests/tcg/mips/Makefile.target | 6 ++++-
> tests/tcg/mips64/Makefile.target | 15 +++++++++++
> tests/tcg/mips64el/Makefile.target | 15 +++++++++++
> tests/tcg/mipsel/Makefile.target | 15 +++++++++++
> tests/tcg/multiarch/Makefile.target | 22 ++++++++++++++--
> .../{ => plugin}/check-plugin-output.sh | 0
> .../{ => plugin}/test-plugin-mem-access.c | 0
> .../plugin/test-plugin-skip-syscalls.c | 26 +++++++++++++++++++
> tests/tcg/plugins/syscall.c | 6 +++++
> tests/tcg/sparc64/Makefile.target | 16 ++++++++++++
> 12 files changed, 131 insertions(+), 3 deletions(-)
> create mode 100644 tests/tcg/mips64/Makefile.target
> create mode 100644 tests/tcg/mips64el/Makefile.target
> create mode 100644 tests/tcg/mipsel/Makefile.target
> rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
> rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
> create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
> create mode 100644 tests/tcg/sparc64/Makefile.target
>
> diff --git a/tests/tcg/arm/Makefile.target b/tests/tcg/arm/Makefile.target
> index 6189d7a0e2..0d8be9cd80 100644
> --- a/tests/tcg/arm/Makefile.target
> +++ b/tests/tcg/arm/Makefile.target
> @@ -78,4 +78,10 @@ sha512-vector: sha512.c
>
> ARM_TESTS += sha512-vector
>
> +ifeq ($(CONFIG_PLUGIN),y)
> +# Require emitting arm32 instructions, otherwise the vCPU might accidentally
> +# try to execute Thumb instructions in arm32 mode after qemu_plugin_set_pc()
> +test-plugin-skip-syscalls: CFLAGS+=-marm
> +endif
> +
> TESTS += $(ARM_TESTS)
> diff --git a/tests/tcg/hexagon/Makefile.target b/tests/tcg/hexagon/Makefile.target
> index f86f02bb31..111dc405fa 100644
> --- a/tests/tcg/hexagon/Makefile.target
> +++ b/tests/tcg/hexagon/Makefile.target
> @@ -126,3 +126,10 @@ v73_scalar: CFLAGS += -Wno-unused-function
>
> hvx_histogram: hvx_histogram.c hvx_histogram_row.S
> $(CC) $(CFLAGS) $(CROSS_CC_GUEST_CFLAGS) $^ -o $@ $(LDFLAGS)
> +
> +ifeq ($(CONFIG_PLUGIN),y)
> +# hexagon uses clang/lld which does not support -Ttext-segment but GNU ld does
> +# not generally support --image-base. Therefore, the multiarch Makefile uses
> +# the GNU ld flag and we special-case here for hexagon.
> +override LDFLAG_TEXT_BASE = -Wl,--image-base=0x40000
> +endif
> diff --git a/tests/tcg/mips/Makefile.target b/tests/tcg/mips/Makefile.target
> index 5d17c1706e..d08138f17b 100644
> --- a/tests/tcg/mips/Makefile.target
> +++ b/tests/tcg/mips/Makefile.target
> @@ -9,11 +9,15 @@ MIPS_SRC=$(SRC_PATH)/tests/tcg/mips
> VPATH += $(MIPS_SRC)
>
> # hello-mips is 32 bit only
> -ifeq ($(findstring 64,$(TARGET_NAME)),)
> MIPS_TESTS=hello-mips
>
> TESTS += $(MIPS_TESTS)
>
> hello-mips: CFLAGS+=-mno-abicalls -fno-PIC -fno-stack-protector -mabi=32
> hello-mips: LDFLAGS+=-nostdlib
> +
> +ifeq ($(CONFIG_PLUGIN),y)
> +# qemu-mips(el) returns ENOSYS without triggering syscall plugin callbacks
> +run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
> + $(call skip-test, $<, "qemu-mips does not execute invalid syscalls")
> endif
> diff --git a/tests/tcg/mips64/Makefile.target b/tests/tcg/mips64/Makefile.target
> new file mode 100644
> index 0000000000..5386855efc
> --- /dev/null
> +++ b/tests/tcg/mips64/Makefile.target
> @@ -0,0 +1,15 @@
> +# -*- Mode: makefile -*-
> +#
> +# MIPS64 - included from tests/tcg/Makefile.target
> +#
> +
> +MIPS64_SRC=$(SRC_PATH)/tests/tcg/mips64
> +
> +# Set search path for all sources
> +VPATH += $(MIPS64_SRC)
> +
> +ifeq ($(CONFIG_PLUGIN),y)
> +# Require no ABI calls to avoid $t9-relative .got address calculation on MIPS64
> +test-plugin-skip-syscalls: CFLAGS+=-mno-abicalls -fno-pie
> +test-plugin-skip-syscalls: LDFLAGS+=-no-pie
> +endif
> diff --git a/tests/tcg/mips64el/Makefile.target b/tests/tcg/mips64el/Makefile.target
> new file mode 100644
> index 0000000000..77ac8815fe
> --- /dev/null
> +++ b/tests/tcg/mips64el/Makefile.target
> @@ -0,0 +1,15 @@
> +# -*- Mode: makefile -*-
> +#
> +# MIPS64EL - included from tests/tcg/Makefile.target
> +#
> +
> +MIPS64EL_SRC=$(SRC_PATH)/tests/tcg/mips64el
> +
> +# Set search path for all sources
> +VPATH += $(MIPS64EL_SRC)
> +
> +ifeq ($(CONFIG_PLUGIN),y)
> +# Require no ABI calls to avoid $t9-relative .got address calculation on MIPS64
> +test-plugin-skip-syscalls: CFLAGS+=-mno-abicalls -fno-pie
> +test-plugin-skip-syscalls: LDFLAGS+=-no-pie
> +endif
> diff --git a/tests/tcg/mipsel/Makefile.target b/tests/tcg/mipsel/Makefile.target
> new file mode 100644
> index 0000000000..bf1bdb56b3
> --- /dev/null
> +++ b/tests/tcg/mipsel/Makefile.target
> @@ -0,0 +1,15 @@
> +# -*- Mode: makefile -*-
> +#
> +# MIPSEL - included from tests/tcg/Makefile.target
> +#
> +
> +MIPSEL_SRC=$(SRC_PATH)/tests/tcg/mipsel
> +
> +# Set search path for all sources
> +VPATH += $(MIPSEL_SRC)
> +
> +ifeq ($(CONFIG_PLUGIN),y)
> +# qemu-mips(el) returns ENOSYS without triggering syscall plugin callbacks
> +run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
> + $(call skip-test, $<, "qemu-mipsel does not execute invalid syscalls")
> +endif
> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
> index 07d0b27bdd..2e2dcda425 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,27 @@ 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:
>
> EXTRA_RUNS_WITH_PLUGIN += run-plugin-test-plugin-mem-access-with-libmem.so \
> run-plugin-test-plugin-syscall-filter-with-libsyscall.so
> -else
> +
> +# Test plugin control flow redirection by skipping system calls
> +# (similar functionality to syscall filter but different mechanism)
> +LDFLAG_TEXT_BASE = -Wl,-Ttext-segment=0x40000
> +test-plugin-skip-syscalls: LDFLAGS += $(LDFLAG_TEXT_BASE)
> +test-plugin-skip-syscalls: LDFLAGS += -Wl,--section-start,.redirect=0x20000
> +run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
> +
> +EXTRA_RUNS_WITH_PLUGIN += run-plugin-test-plugin-skip-syscalls-with-libsyscall.so
> +
> +else # CONFIG_PLUGIN=n
> +# Do not build the syscall skipping test if it's not tested with a plugin
> +# because it will simply return an error and fail the test.
> +MULTIARCH_TESTS := $(filter-out test-plugin-skip-syscalls, $(MULTIARCH_TESTS))
> +
> # 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-skip-syscalls.c b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
> new file mode 100644
> index 0000000000..1f5cbc3851
> --- /dev/null
> +++ b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
> @@ -0,0 +1,26 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This test attempts to execute an invalid syscall. The syscall test plugin
> + * should intercept this.
> + */
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +
> +void exit_success(void) __attribute__((section(".redirect"), noinline,
> + noreturn, used));
> +
> +void exit_success(void) {
> + _exit(EXIT_SUCCESS);
> +}
> +
> +int main(int argc, char *argv[]) {
> + long ret = syscall(0xc0deUL);
> + if (ret != 0L) {
> + perror("");
> + }
> + /* We should never get here */
> + return EXIT_FAILURE;
> +}
I'm running into an issue for all four variants of MIPS if I don't
hardcode the section but pass the function address as a syscall argument
and then use that as jump target in the plugin: according to the ABI,
the t9 register has to contain the address of the function being called.
The function prologue then calculates the gp register value (global
pointer / context pointer) based on t9, and derives the new values of t9
for any callees from gp again. As I'm currently just updating the pc
with the new API function, t9 is out of sync with the code after control
flow redirection and the binary crashes.
I think it is fair to expect a user of the API to be aware of such
pitfalls (or we can document it), but I'd of course still like to make
the tests pass. The simplest solution (theoretically) is to also set the
t9 register in the plugin callback before calling qemu_plugin_set_pc.
However, the MIPS targets do not actually expose any registers to
plugins, i.e., qemu_plugin_get_registers returns an empty GArray.
Given this behavior, I see two solutions:
1) skipping the test on MIPS, or
2) making the test code a bit more contrived to use labels within the
same function while preventing the compiler from optimizing the
labels away (which it does even with -O0). I've got a prototype for
this, but the test code looks a bit contrived then:
int main(int argc, char *argv[]) {
int retvals[] = {EXIT_SUCCESS, EXIT_FAILURE};
int retval_idx = 1;
long ret = syscall(0xc0deUL, &&good);
if (ret < 0) {
perror("");
goto bad;
} else if (ret == 0xdeadbeefUL) {
/*
* Should never arrive here but we need this nevertheless to prevent
* the compiler from optimizing away the label. Otherwise, the compiler
* silently rewrites the label value used in the syscall to another
* address (typically pointing to right after the function prologue).
*/
printf("Check what's wrong, we should never arrive here!");
assert(((uintptr_t)&&good == (uintptr_t)ret));
/* We should absolutely never arrive here, the assert should trigger */
goto good;
}
bad:
retval_idx = 1;
goto exit;
good:
retval_idx = 0;
exit:
return retvals[retval_idx];
}
Maybe I'm just missing something obvious, I'd be happy to get some
feedback on this. Thanks in advance!
> diff --git a/tests/tcg/plugins/syscall.c b/tests/tcg/plugins/syscall.c
> index 5658f83087..b68e3cadf4 100644
> --- a/tests/tcg/plugins/syscall.c
> +++ b/tests/tcg/plugins/syscall.c
> @@ -148,6 +148,12 @@ static void vcpu_syscall(qemu_plugin_id_t id, unsigned int vcpu_index,
> fprintf(stderr, "Error reading memory from vaddr %"PRIu64"\n", a2);
> }
> }
> +
> + if (num == 0xc0deUL) {
> + /* Special syscall to test the control flow redirection functionality. */
> + qemu_plugin_outs("Marker syscall detected, jump to clean exit\n");
> + qemu_plugin_set_pc(0x20000);
> + }
> }
>
> static void vcpu_syscall_ret(qemu_plugin_id_t id, unsigned int vcpu_idx,
> diff --git a/tests/tcg/sparc64/Makefile.target b/tests/tcg/sparc64/Makefile.target
> new file mode 100644
> index 0000000000..516927a3fc
> --- /dev/null
> +++ b/tests/tcg/sparc64/Makefile.target
> @@ -0,0 +1,16 @@
> +# -*- Mode: makefile -*-
> +#
> +# Sparc64 - included from tests/tcg/Makefile.target
> +#
> +
> +SPARC64_SRC=$(SRC_PATH)/tests/tcg/sparc64
> +
> +# Set search path for all sources
> +VPATH += $(SPARC64_SRC)
> +
> +ifeq ($(CONFIG_PLUGIN),y)
> +# The defined addresses for the binary are not aligned correctly for sparc64
> +# but adjusting them breaks other architectures, so just skip it on sparc64.
> +run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
> + $(call skip-test, $<, "qemu-sparc64 does not allow mapping at our given fixed address")
> +endif
Best regards,
Florian
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4346 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API
2026-02-25 16:21 ` Florian Hofhammer
@ 2026-02-25 17:30 ` Pierrick Bouvier
2026-02-25 17:39 ` Pierrick Bouvier
2026-02-26 8:30 ` Florian Hofhammer
0 siblings, 2 replies; 46+ messages in thread
From: Pierrick Bouvier @ 2026-02-25 17:30 UTC (permalink / raw)
To: Florian Hofhammer, alex.bennee
Cc: richard.henderson, laurent, imp, berrange, qemu-devel
On 2/25/26 8:21 AM, Florian Hofhammer wrote:
> On 24/02/2026 16:52, Florian Hofhammer wrote:
>> The test executes a non-existent syscall, which the syscall plugin
>> intercepts and redirects to a clean exit.
>> Due to architecture-specific quirks, the architecture-specific Makefiles
>> require setting specific compiler and linker flags in some cases.
>>
>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>> ---
>> tests/tcg/arm/Makefile.target | 6 +++++
>> tests/tcg/hexagon/Makefile.target | 7 +++++
>> tests/tcg/mips/Makefile.target | 6 ++++-
>> tests/tcg/mips64/Makefile.target | 15 +++++++++++
>> tests/tcg/mips64el/Makefile.target | 15 +++++++++++
>> tests/tcg/mipsel/Makefile.target | 15 +++++++++++
>> tests/tcg/multiarch/Makefile.target | 22 ++++++++++++++--
>> .../{ => plugin}/check-plugin-output.sh | 0
>> .../{ => plugin}/test-plugin-mem-access.c | 0
>> .../plugin/test-plugin-skip-syscalls.c | 26 +++++++++++++++++++
>> tests/tcg/plugins/syscall.c | 6 +++++
>> tests/tcg/sparc64/Makefile.target | 16 ++++++++++++
>> 12 files changed, 131 insertions(+), 3 deletions(-)
>> create mode 100644 tests/tcg/mips64/Makefile.target
>> create mode 100644 tests/tcg/mips64el/Makefile.target
>> create mode 100644 tests/tcg/mipsel/Makefile.target
>> rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
>> rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
>> create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>> create mode 100644 tests/tcg/sparc64/Makefile.target
>>
>> diff --git a/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>> new file mode 100644
>> index 0000000000..1f5cbc3851
>> --- /dev/null
>> +++ b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>> @@ -0,0 +1,26 @@
>> +/*
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + *
>> + * This test attempts to execute an invalid syscall. The syscall test plugin
>> + * should intercept this.
>> + */
>> +#include <stdint.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <unistd.h>
>> +
>> +void exit_success(void) __attribute__((section(".redirect"), noinline,
>> + noreturn, used));
>> +
>> +void exit_success(void) {
>> + _exit(EXIT_SUCCESS);
>> +}
>> +
>> +int main(int argc, char *argv[]) {
>> + long ret = syscall(0xc0deUL);
>> + if (ret != 0L) {
>> + perror("");
>> + }
>> + /* We should never get here */
>> + return EXIT_FAILURE;
>> +}
>
> I'm running into an issue for all four variants of MIPS if I don't
> hardcode the section but pass the function address as a syscall argument
> and then use that as jump target in the plugin: according to the ABI,
> the t9 register has to contain the address of the function being called.
> The function prologue then calculates the gp register value (global
> pointer / context pointer) based on t9, and derives the new values of t9
> for any callees from gp again. As I'm currently just updating the pc
> with the new API function, t9 is out of sync with the code after control
> flow redirection and the binary crashes.
>> I think it is fair to expect a user of the API to be aware of such
> pitfalls (or we can document it), but I'd of course still like to make
> the tests pass. The simplest solution (theoretically) is to also set the
> t9 register in the plugin callback before calling qemu_plugin_set_pc.
> However, the MIPS targets do not actually expose any registers to
> plugins, i.e., qemu_plugin_get_registers returns an empty GArray.
>
A lot of things can go wrong when jumping to a different context than
the current one, that's why setjmp/longjmp exist. You can always add a
comment for this "This function only changes pc and does not guarantee
other registers representing context will have a proper value when
updating it".
A reason why I pushed for using value labels, was to stay in the same
context, and avoid the kind of issues you ran into.
> Given this behavior, I see two solutions:
> 1) skipping the test on MIPS, or
> 2) making the test code a bit more contrived to use labels within the
> same function while preventing the compiler from optimizing the
> labels away (which it does even with -O0). I've got a prototype for
> this, but the test code looks a bit contrived then:
>
> int main(int argc, char *argv[]) {
> int retvals[] = {EXIT_SUCCESS, EXIT_FAILURE};
> int retval_idx = 1;
>
> long ret = syscall(0xc0deUL, &&good);
> if (ret < 0) {
> perror("");
> goto bad;
> } else if (ret == 0xdeadbeefUL) {
> /*
> * Should never arrive here but we need this nevertheless to prevent
> * the compiler from optimizing away the label. Otherwise, the compiler
> * silently rewrites the label value used in the syscall to another
> * address (typically pointing to right after the function prologue).
> */
> printf("Check what's wrong, we should never arrive here!");
> assert(((uintptr_t)&&good == (uintptr_t)ret));
> /* We should absolutely never arrive here, the assert should trigger */
> goto good;
> }
>
> bad:
> retval_idx = 1;
> goto exit;
> good:
> retval_idx = 0;
> exit:
> return retvals[retval_idx];
> }
>
> Maybe I'm just missing something obvious, I'd be happy to get some
> feedback on this. Thanks in advance!
>
Can you post the exact code you had where labels are optimized away?
On which arch was it?
I tried something similar but didn't see the second one disappear.
I wonder if it's related to compiler detecting g_assert_not_reached() is
a "noreturn" function, thus it doesn't expect to go past it, and
eliminates dead code. You can try with assert(0) or moving
g_assert_not_reached() to another function "crash()" instead, that is
not marked as noreturn.
> Best regards,
> Florian
Regards,
Pierrick
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API
2026-02-25 17:30 ` Pierrick Bouvier
@ 2026-02-25 17:39 ` Pierrick Bouvier
2026-02-26 8:30 ` Florian Hofhammer
1 sibling, 0 replies; 46+ messages in thread
From: Pierrick Bouvier @ 2026-02-25 17:39 UTC (permalink / raw)
To: Florian Hofhammer, alex.bennee
Cc: richard.henderson, laurent, imp, berrange, qemu-devel
On 2/25/26 9:30 AM, Pierrick Bouvier wrote:
> On 2/25/26 8:21 AM, Florian Hofhammer wrote:
>> On 24/02/2026 16:52, Florian Hofhammer wrote:
>>> The test executes a non-existent syscall, which the syscall plugin
>>> intercepts and redirects to a clean exit.
>>> Due to architecture-specific quirks, the architecture-specific Makefiles
>>> require setting specific compiler and linker flags in some cases.
>>>
>>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>>> ---
>>> tests/tcg/arm/Makefile.target | 6 +++++
>>> tests/tcg/hexagon/Makefile.target | 7 +++++
>>> tests/tcg/mips/Makefile.target | 6 ++++-
>>> tests/tcg/mips64/Makefile.target | 15 +++++++++++
>>> tests/tcg/mips64el/Makefile.target | 15 +++++++++++
>>> tests/tcg/mipsel/Makefile.target | 15 +++++++++++
>>> tests/tcg/multiarch/Makefile.target | 22 ++++++++++++++--
>>> .../{ => plugin}/check-plugin-output.sh | 0
>>> .../{ => plugin}/test-plugin-mem-access.c | 0
>>> .../plugin/test-plugin-skip-syscalls.c | 26 +++++++++++++++++++
>>> tests/tcg/plugins/syscall.c | 6 +++++
>>> tests/tcg/sparc64/Makefile.target | 16 ++++++++++++
>>> 12 files changed, 131 insertions(+), 3 deletions(-)
>>> create mode 100644 tests/tcg/mips64/Makefile.target
>>> create mode 100644 tests/tcg/mips64el/Makefile.target
>>> create mode 100644 tests/tcg/mipsel/Makefile.target
>>> rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
>>> rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
>>> create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>>> create mode 100644 tests/tcg/sparc64/Makefile.target
>>>
>>> diff --git a/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>>> new file mode 100644
>>> index 0000000000..1f5cbc3851
>>> --- /dev/null
>>> +++ b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>>> @@ -0,0 +1,26 @@
>>> +/*
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + *
>>> + * This test attempts to execute an invalid syscall. The syscall test plugin
>>> + * should intercept this.
>>> + */
>>> +#include <stdint.h>
>>> +#include <stdio.h>
>>> +#include <stdlib.h>
>>> +#include <unistd.h>
>>> +
>>> +void exit_success(void) __attribute__((section(".redirect"), noinline,
>>> + noreturn, used));
>>> +
>>> +void exit_success(void) {
>>> + _exit(EXIT_SUCCESS);
>>> +}
>>> +
>>> +int main(int argc, char *argv[]) {
>>> + long ret = syscall(0xc0deUL);
>>> + if (ret != 0L) {
>>> + perror("");
>>> + }
>>> + /* We should never get here */
>>> + return EXIT_FAILURE;
>>> +}
>>
>> I'm running into an issue for all four variants of MIPS if I don't
>> hardcode the section but pass the function address as a syscall argument
>> and then use that as jump target in the plugin: according to the ABI,
>> the t9 register has to contain the address of the function being called.
>> The function prologue then calculates the gp register value (global
>> pointer / context pointer) based on t9, and derives the new values of t9
>> for any callees from gp again. As I'm currently just updating the pc
>> with the new API function, t9 is out of sync with the code after control
>> flow redirection and the binary crashes.
>>> I think it is fair to expect a user of the API to be aware of such
>> pitfalls (or we can document it), but I'd of course still like to make
>> the tests pass. The simplest solution (theoretically) is to also set the
>> t9 register in the plugin callback before calling qemu_plugin_set_pc.
>> However, the MIPS targets do not actually expose any registers to
>> plugins, i.e., qemu_plugin_get_registers returns an empty GArray.
>>
>
> A lot of things can go wrong when jumping to a different context than
> the current one, that's why setjmp/longjmp exist. You can always add a
> comment for this "This function only changes pc and does not guarantee
> other registers representing context will have a proper value when
> updating it".
>
> A reason why I pushed for using value labels, was to stay in the same
> context, and avoid the kind of issues you ran into.
>
>> Given this behavior, I see two solutions:
>> 1) skipping the test on MIPS, or
>> 2) making the test code a bit more contrived to use labels within the
>> same function while preventing the compiler from optimizing the
>> labels away (which it does even with -O0). I've got a prototype for
>> this, but the test code looks a bit contrived then:
>>
>> int main(int argc, char *argv[]) {
>> int retvals[] = {EXIT_SUCCESS, EXIT_FAILURE};
>> int retval_idx = 1;
>>
>> long ret = syscall(0xc0deUL, &&good);
>> if (ret < 0) {
>> perror("");
>> goto bad;
>> } else if (ret == 0xdeadbeefUL) {
>> /*
>> * Should never arrive here but we need this nevertheless to prevent
>> * the compiler from optimizing away the label. Otherwise, the compiler
>> * silently rewrites the label value used in the syscall to another
>> * address (typically pointing to right after the function prologue).
>> */
>> printf("Check what's wrong, we should never arrive here!");
>> assert(((uintptr_t)&&good == (uintptr_t)ret));
>> /* We should absolutely never arrive here, the assert should trigger */
>> goto good;
>> }
>>
>> bad:
>> retval_idx = 1;
>> goto exit;
>> good:
>> retval_idx = 0;
>> exit:
>> return retvals[retval_idx];
>> }
>>
>> Maybe I'm just missing something obvious, I'd be happy to get some
>> feedback on this. Thanks in advance!
>>
>
> Can you post the exact code you had where labels are optimized away?
> On which arch was it?
> I tried something similar but didn't see the second one disappear.
>
> I wonder if it's related to compiler detecting g_assert_not_reached() is
> a "noreturn" function, thus it doesn't expect to go past it, and
> eliminates dead code. You can try with assert(0) or moving
> g_assert_not_reached() to another function "crash()" instead, that is
> not marked as noreturn.
>
I'm pretty sure that's the reason:
Given this code:
void crash(void)
{
g_assert_not_reached();
}
int main() {
crash();
printf("Hello World\n");
}
resulting assembly contains a call to puts.
Meanwhile:
int main() {
g_assert_not_reached();
printf("Hello World\n");
}
Doesn't.
gcc does dead code elimination even in 0 for cases like this, or if you
have if (0) { ... } also. Beyond code size optimization purpose, it's
useful to remove dependency to symbols that are really in dead code, I
discovered recently we rely on that behaviour in QEMU (for various
X_enabled global symbols).
>> Best regards,
>> Florian
>
> Regards,
> Pierrick
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API
2026-02-25 17:30 ` Pierrick Bouvier
2026-02-25 17:39 ` Pierrick Bouvier
@ 2026-02-26 8:30 ` Florian Hofhammer
2026-02-26 19:47 ` Pierrick Bouvier
1 sibling, 1 reply; 46+ messages in thread
From: Florian Hofhammer @ 2026-02-26 8:30 UTC (permalink / raw)
To: Pierrick Bouvier, alex.bennee
Cc: richard.henderson, laurent, imp, berrange, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 8970 bytes --]
On 25/02/2026 18:30, Pierrick Bouvier wrote:
> On 2/25/26 8:21 AM, Florian Hofhammer wrote:
>> On 24/02/2026 16:52, Florian Hofhammer wrote:
>>> The test executes a non-existent syscall, which the syscall plugin
>>> intercepts and redirects to a clean exit.
>>> Due to architecture-specific quirks, the architecture-specific Makefiles
>>> require setting specific compiler and linker flags in some cases.
>>>
>>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>>> ---
>>> tests/tcg/arm/Makefile.target | 6 +++++
>>> tests/tcg/hexagon/Makefile.target | 7 +++++
>>> tests/tcg/mips/Makefile.target | 6 ++++-
>>> tests/tcg/mips64/Makefile.target | 15 +++++++++++
>>> tests/tcg/mips64el/Makefile.target | 15 +++++++++++
>>> tests/tcg/mipsel/Makefile.target | 15 +++++++++++
>>> tests/tcg/multiarch/Makefile.target | 22 ++++++++++++++--
>>> .../{ => plugin}/check-plugin-output.sh | 0
>>> .../{ => plugin}/test-plugin-mem-access.c | 0
>>> .../plugin/test-plugin-skip-syscalls.c | 26 +++++++++++++++++++
>>> tests/tcg/plugins/syscall.c | 6 +++++
>>> tests/tcg/sparc64/Makefile.target | 16 ++++++++++++
>>> 12 files changed, 131 insertions(+), 3 deletions(-)
>>> create mode 100644 tests/tcg/mips64/Makefile.target
>>> create mode 100644 tests/tcg/mips64el/Makefile.target
>>> create mode 100644 tests/tcg/mipsel/Makefile.target
>>> rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
>>> rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
>>> create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>>> create mode 100644 tests/tcg/sparc64/Makefile.target
>>>
>>> diff --git a/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>>> new file mode 100644
>>> index 0000000000..1f5cbc3851
>>> --- /dev/null
>>> +++ b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>>> @@ -0,0 +1,26 @@
>>> +/*
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + *
>>> + * This test attempts to execute an invalid syscall. The syscall test plugin
>>> + * should intercept this.
>>> + */
>>> +#include <stdint.h>
>>> +#include <stdio.h>
>>> +#include <stdlib.h>
>>> +#include <unistd.h>
>>> +
>>> +void exit_success(void) __attribute__((section(".redirect"), noinline,
>>> + noreturn, used));
>>> +
>>> +void exit_success(void) {
>>> + _exit(EXIT_SUCCESS);
>>> +}
>>> +
>>> +int main(int argc, char *argv[]) {
>>> + long ret = syscall(0xc0deUL);
>>> + if (ret != 0L) {
>>> + perror("");
>>> + }
>>> + /* We should never get here */
>>> + return EXIT_FAILURE;
>>> +}
>>
>> I'm running into an issue for all four variants of MIPS if I don't
>> hardcode the section but pass the function address as a syscall argument
>> and then use that as jump target in the plugin: according to the ABI,
>> the t9 register has to contain the address of the function being called.
>> The function prologue then calculates the gp register value (global
>> pointer / context pointer) based on t9, and derives the new values of t9
>> for any callees from gp again. As I'm currently just updating the pc
>> with the new API function, t9 is out of sync with the code after control
>> flow redirection and the binary crashes.
>>> I think it is fair to expect a user of the API to be aware of such
>> pitfalls (or we can document it), but I'd of course still like to make
>> the tests pass. The simplest solution (theoretically) is to also set the
>> t9 register in the plugin callback before calling qemu_plugin_set_pc.
>> However, the MIPS targets do not actually expose any registers to
>> plugins, i.e., qemu_plugin_get_registers returns an empty GArray.
>>
>
> A lot of things can go wrong when jumping to a different context than the current one, that's why setjmp/longjmp exist. You can always add a comment for this "This function only changes pc and does not guarantee other registers representing context will have a proper value when updating it".
>
> A reason why I pushed for using value labels, was to stay in the same context, and avoid the kind of issues you ran into.
>
>> Given this behavior, I see two solutions:
>> 1) skipping the test on MIPS, or
>> 2) making the test code a bit more contrived to use labels within the
>> same function while preventing the compiler from optimizing the
>> labels away (which it does even with -O0). I've got a prototype for
>> this, but the test code looks a bit contrived then:
>>
>> int main(int argc, char *argv[]) {
>> int retvals[] = {EXIT_SUCCESS, EXIT_FAILURE};
>> int retval_idx = 1;
>>
>> long ret = syscall(0xc0deUL, &&good);
>> if (ret < 0) {
>> perror("");
>> goto bad;
>> } else if (ret == 0xdeadbeefUL) {
>> /*
>> * Should never arrive here but we need this nevertheless to prevent
>> * the compiler from optimizing away the label. Otherwise, the compiler
>> * silently rewrites the label value used in the syscall to another
>> * address (typically pointing to right after the function prologue).
>> */
>> printf("Check what's wrong, we should never arrive here!");
>> assert(((uintptr_t)&&good == (uintptr_t)ret));
>> /* We should absolutely never arrive here, the assert should trigger */
>> goto good;
>> }
>>
>> bad:
>> retval_idx = 1;
>> goto exit;
>> good:
>> retval_idx = 0;
>> exit:
>> return retvals[retval_idx];
>> }
>>
>> Maybe I'm just missing something obvious, I'd be happy to get some
>> feedback on this. Thanks in advance!
>>
>
> Can you post the exact code you had where labels are optimized away?
> On which arch was it?
This happens across architectures but I've verified with x86, aarch64,
and riscv64. Below the C code and generated assembly, compiled with gcc
-O0 -o test.s -S test.c (adding -fno-dce and -fno-tree-dce does not
change anything and they should be included in -O0 anyway):
test.c:
#include <stdlib.h>
#include <unistd.h>
int main(int argc, char *argv[]) {
syscall(4096, &&good);
return EXIT_FAILURE;
good:
return EXIT_SUCCESS;
}
test.s:
.file "test.c"
.text
.globl main
.type main, @function
main:
.LFB6:
.cfi_startproc
pushq %rbp
.cfi_def_cfa_offset 16
.cfi_offset 6, -16
movq %rsp, %rbp
.cfi_def_cfa_register 6
subq $16, %rsp
movl %edi, -4(%rbp)
movq %rsi, -16(%rbp)
.L2:
leaq .L2(%rip), %rax
movq %rax, %rsi
movl $4096, %edi
movl $0, %eax
call syscall@PLT
movl $1, %eax
leave
.cfi_def_cfa 7, 8
ret
.cfi_endproc
.LFE6:
.size main, .-main
.ident "GCC: (GNU) 15.2.1 20260209"
.section .note.GNU-stack,"",@progbits
As you can see, the "good" label and correspondingly the second return
are optimized away, and the compiler replaces the syscall argument
&&good with the address of the .L2 assembly label. This effectively then
causes an infinite loop of the syscall being called over and over again
and redirecting the PC to right after the function prologue.
The assembly output for aarch64 and riscv64 is analogous.
> I tried something similar but didn't see the second one disappear.
>
> I wonder if it's related to compiler detecting g_assert_not_reached() is a "noreturn" function, thus it doesn't expect to go past it, and eliminates dead code. You can try with assert(0) or moving g_assert_not_reached() to another function "crash()" instead, that is not marked as noreturn.
As I mentioned above, even turning off dead code elimination didn't
resolve the issue. But this was actually a good pointer nevertheless:
just wrapping the exit in a separate function like below solves the
issue. It's just that the compiler detected that the label is after a
return and deemed it unreachable (obviously not knowing about the
semantics of the syscall). Below is the modified code that works across
architectures and that I'll use in the end:
void exit_failure(void) {
_exit(EXIT_FAILURE);
}
void exit_success(void) {
_exit(EXIT_SUCCESS);
}
int main(int argc, char *argv[]) {
syscall(4096, &&good);
exit_failure();
good:
exit_success();
}
>
>> Best regards,
>> Florian
>
> Regards,
> Pierrick
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4346 bytes --]
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API
2026-02-26 8:30 ` Florian Hofhammer
@ 2026-02-26 19:47 ` Pierrick Bouvier
0 siblings, 0 replies; 46+ messages in thread
From: Pierrick Bouvier @ 2026-02-26 19:47 UTC (permalink / raw)
To: Florian Hofhammer, alex.bennee
Cc: richard.henderson, laurent, imp, berrange, qemu-devel
On 2/26/26 12:30 AM, Florian Hofhammer wrote:
> On 25/02/2026 18:30, Pierrick Bouvier wrote:
>> On 2/25/26 8:21 AM, Florian Hofhammer wrote:
>>> On 24/02/2026 16:52, Florian Hofhammer wrote:
>>>> The test executes a non-existent syscall, which the syscall plugin
>>>> intercepts and redirects to a clean exit.
>>>> Due to architecture-specific quirks, the architecture-specific Makefiles
>>>> require setting specific compiler and linker flags in some cases.
>>>>
>>>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>>>> ---
>>>> tests/tcg/arm/Makefile.target | 6 +++++
>>>> tests/tcg/hexagon/Makefile.target | 7 +++++
>>>> tests/tcg/mips/Makefile.target | 6 ++++-
>>>> tests/tcg/mips64/Makefile.target | 15 +++++++++++
>>>> tests/tcg/mips64el/Makefile.target | 15 +++++++++++
>>>> tests/tcg/mipsel/Makefile.target | 15 +++++++++++
>>>> tests/tcg/multiarch/Makefile.target | 22 ++++++++++++++--
>>>> .../{ => plugin}/check-plugin-output.sh | 0
>>>> .../{ => plugin}/test-plugin-mem-access.c | 0
>>>> .../plugin/test-plugin-skip-syscalls.c | 26 +++++++++++++++++++
>>>> tests/tcg/plugins/syscall.c | 6 +++++
>>>> tests/tcg/sparc64/Makefile.target | 16 ++++++++++++
>>>> 12 files changed, 131 insertions(+), 3 deletions(-)
>>>> create mode 100644 tests/tcg/mips64/Makefile.target
>>>> create mode 100644 tests/tcg/mips64el/Makefile.target
>>>> create mode 100644 tests/tcg/mipsel/Makefile.target
>>>> rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
>>>> rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
>>>> create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>>>> create mode 100644 tests/tcg/sparc64/Makefile.target
>>>>
>>>> diff --git a/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>>>> new file mode 100644
>>>> index 0000000000..1f5cbc3851
>>>> --- /dev/null
>>>> +++ b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
>>>> @@ -0,0 +1,26 @@
>>>> +/*
>>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>>> + *
>>>> + * This test attempts to execute an invalid syscall. The syscall test plugin
>>>> + * should intercept this.
>>>> + */
>>>> +#include <stdint.h>
>>>> +#include <stdio.h>
>>>> +#include <stdlib.h>
>>>> +#include <unistd.h>
>>>> +
>>>> +void exit_success(void) __attribute__((section(".redirect"), noinline,
>>>> + noreturn, used));
>>>> +
>>>> +void exit_success(void) {
>>>> + _exit(EXIT_SUCCESS);
>>>> +}
>>>> +
>>>> +int main(int argc, char *argv[]) {
>>>> + long ret = syscall(0xc0deUL);
>>>> + if (ret != 0L) {
>>>> + perror("");
>>>> + }
>>>> + /* We should never get here */
>>>> + return EXIT_FAILURE;
>>>> +}
>>>
>>> I'm running into an issue for all four variants of MIPS if I don't
>>> hardcode the section but pass the function address as a syscall argument
>>> and then use that as jump target in the plugin: according to the ABI,
>>> the t9 register has to contain the address of the function being called.
>>> The function prologue then calculates the gp register value (global
>>> pointer / context pointer) based on t9, and derives the new values of t9
>>> for any callees from gp again. As I'm currently just updating the pc
>>> with the new API function, t9 is out of sync with the code after control
>>> flow redirection and the binary crashes.
>>>> I think it is fair to expect a user of the API to be aware of such
>>> pitfalls (or we can document it), but I'd of course still like to make
>>> the tests pass. The simplest solution (theoretically) is to also set the
>>> t9 register in the plugin callback before calling qemu_plugin_set_pc.
>>> However, the MIPS targets do not actually expose any registers to
>>> plugins, i.e., qemu_plugin_get_registers returns an empty GArray.
>>>
>>
>> A lot of things can go wrong when jumping to a different context than the current one, that's why setjmp/longjmp exist. You can always add a comment for this "This function only changes pc and does not guarantee other registers representing context will have a proper value when updating it".
>>
>> A reason why I pushed for using value labels, was to stay in the same context, and avoid the kind of issues you ran into.
>>
>>> Given this behavior, I see two solutions:
>>> 1) skipping the test on MIPS, or
>>> 2) making the test code a bit more contrived to use labels within the
>>> same function while preventing the compiler from optimizing the
>>> labels away (which it does even with -O0). I've got a prototype for
>>> this, but the test code looks a bit contrived then:
>>>
>>> int main(int argc, char *argv[]) {
>>> int retvals[] = {EXIT_SUCCESS, EXIT_FAILURE};
>>> int retval_idx = 1;
>>>
>>> long ret = syscall(0xc0deUL, &&good);
>>> if (ret < 0) {
>>> perror("");
>>> goto bad;
>>> } else if (ret == 0xdeadbeefUL) {
>>> /*
>>> * Should never arrive here but we need this nevertheless to prevent
>>> * the compiler from optimizing away the label. Otherwise, the compiler
>>> * silently rewrites the label value used in the syscall to another
>>> * address (typically pointing to right after the function prologue).
>>> */
>>> printf("Check what's wrong, we should never arrive here!");
>>> assert(((uintptr_t)&&good == (uintptr_t)ret));
>>> /* We should absolutely never arrive here, the assert should trigger */
>>> goto good;
>>> }
>>>
>>> bad:
>>> retval_idx = 1;
>>> goto exit;
>>> good:
>>> retval_idx = 0;
>>> exit:
>>> return retvals[retval_idx];
>>> }
>>>
>>> Maybe I'm just missing something obvious, I'd be happy to get some
>>> feedback on this. Thanks in advance!
>>>
>>
>> Can you post the exact code you had where labels are optimized away?
>> On which arch was it?
>
> This happens across architectures but I've verified with x86, aarch64,
> and riscv64. Below the C code and generated assembly, compiled with gcc
> -O0 -o test.s -S test.c (adding -fno-dce and -fno-tree-dce does not
> change anything and they should be included in -O0 anyway):
>
> test.c:
> #include <stdlib.h>
> #include <unistd.h>
>
> int main(int argc, char *argv[]) {
>
> syscall(4096, &&good);
> return EXIT_FAILURE;
> good:
> return EXIT_SUCCESS;
>
> }
>
> test.s:
> .file "test.c"
> .text
> .globl main
> .type main, @function
> main:
> .LFB6:
> .cfi_startproc
> pushq %rbp
> .cfi_def_cfa_offset 16
> .cfi_offset 6, -16
> movq %rsp, %rbp
> .cfi_def_cfa_register 6
> subq $16, %rsp
> movl %edi, -4(%rbp)
> movq %rsi, -16(%rbp)
> .L2:
> leaq .L2(%rip), %rax
> movq %rax, %rsi
> movl $4096, %edi
> movl $0, %eax
> call syscall@PLT
> movl $1, %eax
> leave
> .cfi_def_cfa 7, 8
> ret
> .cfi_endproc
> .LFE6:
> .size main, .-main
> .ident "GCC: (GNU) 15.2.1 20260209"
> .section .note.GNU-stack,"",@progbits
>
> As you can see, the "good" label and correspondingly the second return
> are optimized away, and the compiler replaces the syscall argument
> &&good with the address of the .L2 assembly label. This effectively then
> causes an infinite loop of the syscall being called over and over again
> and redirecting the PC to right after the function prologue.
> The assembly output for aarch64 and riscv64 is analogous.
>
yes, that's the effect of noreturn attribute. Luckily, gcc is not "smart
enough" at O0 to propage this attribute through function calls.
>> I tried something similar but didn't see the second one disappear.
>>
>> I wonder if it's related to compiler detecting g_assert_not_reached() is a "noreturn" function, thus it doesn't expect to go past it, and eliminates dead code. You can try with assert(0) or moving g_assert_not_reached() to another function "crash()" instead, that is not marked as noreturn.
>
> As I mentioned above, even turning off dead code elimination didn't
> resolve the issue. But this was actually a good pointer nevertheless:
> just wrapping the exit in a separate function like below solves the
> issue. It's just that the compiler detected that the label is after a
> return and deemed it unreachable (obviously not knowing about the
> semantics of the syscall). Below is the modified code that works across
> architectures and that I'll use in the end:
>
> void exit_failure(void) {
> _exit(EXIT_FAILURE);
> }
>
> void exit_success(void) {
> _exit(EXIT_SUCCESS);
> }
>
> int main(int argc, char *argv[]) {
> syscall(4096, &&good);
> exit_failure();
> good:
> exit_success();
> }
>
As mentioned in another comment, you might prefer to have something like:
```
void test_from_syscall() {
syscall(4096, &&good);
exit_failure();
good:
return;
}
void test_from_sighandler() {
...
}
void test_from_instruction() {
...
}
/* So we can combine all tests in a single one */
int main(int argc, char *argv[]) {
test_from_syscall();
test_from_sighandler();
test_from_instruction();
}
```
If that's too complicated for any reason, feel free to have 3 different
tests. It's just nicer to have this, but not mandatory.
>>
>>> Best regards,
>>> Florian
>>
>> Regards,
>> Pierrick
Regards,
Pierrick
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v4 5/7] plugins: add read-only property for registers
2026-02-24 15:46 [PATCH v4 0/7] Enable PC diversion via the plugin API Florian Hofhammer
` (3 preceding siblings ...)
2026-02-24 15:52 ` [PATCH v4 4/7] tests/tcg: add test for qemu_plugin_set_pc API Florian Hofhammer
@ 2026-02-24 15:53 ` Florian Hofhammer
2026-02-24 17:46 ` Alex Bennée
2026-02-26 11:55 ` Florian Hofhammer
2026-02-24 15:57 ` [PATCH v4 6/7] plugins: prohibit writing to read-only registers Florian Hofhammer
` (2 subsequent siblings)
7 siblings, 2 replies; 46+ messages in thread
From: Florian Hofhammer @ 2026-02-24 15:53 UTC (permalink / raw)
To: qemu-devel
Cc: alex.bennee, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
Some registers should be marked as read-only from a plugin API
perspective, as writing to them via qemu_plugin_write_register has no
effect. This includes the program counter, and we expose this fact to
the plugins with this patch.
Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
---
include/plugins/qemu-plugin.h | 2 ++
plugins/api.c | 16 ++++++++++++++++
2 files changed, 18 insertions(+)
diff --git a/include/plugins/qemu-plugin.h b/include/plugins/qemu-plugin.h
index 04c884e82b..ae711758f1 100644
--- a/include/plugins/qemu-plugin.h
+++ b/include/plugins/qemu-plugin.h
@@ -979,11 +979,13 @@ struct qemu_plugin_register;
* writing value with qemu_plugin_write_register
* @name: register name
* @feature: optional feature descriptor, can be NULL
+ * @is_readonly: true if the register cannot be written via qemu_plugin_write_register
*/
typedef struct {
struct qemu_plugin_register *handle;
const char *name;
const char *feature;
+ bool is_readonly;
} qemu_plugin_reg_descriptor;
/**
diff --git a/plugins/api.c b/plugins/api.c
index ca3e93a194..b2c52d2a09 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 program counter
+static const char rip_str[] = "rip"; // x86_64 specific name for program counter
+static const char pswa_str[] = "pswa"; // s390x specific name for program counter
+static const char iaoq_str[] = "iaoq"; // HP/PA specific name for program counter
+static const char rpc_str[] = "rpc"; // microblaze specific name for program counter
static GArray *create_register_handles(GArray *gdbstub_regs)
{
GArray *find_data = g_array_new(true, true,
@@ -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] 46+ messages in thread* Re: [PATCH v4 5/7] plugins: add read-only property for registers
2026-02-24 15:53 ` [PATCH v4 5/7] plugins: add read-only property for registers Florian Hofhammer
@ 2026-02-24 17:46 ` Alex Bennée
2026-02-26 11:55 ` Florian Hofhammer
1 sibling, 0 replies; 46+ messages in thread
From: Alex Bennée @ 2026-02-24 17:46 UTC (permalink / raw)
To: Florian Hofhammer
Cc: qemu-devel, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
Florian Hofhammer <florian.hofhammer@epfl.ch> writes:
> Some registers should be marked as read-only from a plugin API
> perspective, as writing to them via qemu_plugin_write_register has no
> effect. This includes the program counter, and we expose this fact to
> the plugins with this patch.
>
> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v4 5/7] plugins: add read-only property for registers
2026-02-24 15:53 ` [PATCH v4 5/7] plugins: add read-only property for registers Florian Hofhammer
2026-02-24 17:46 ` Alex Bennée
@ 2026-02-26 11:55 ` Florian Hofhammer
2026-02-26 14:33 ` Alex Bennée
1 sibling, 1 reply; 46+ messages in thread
From: Florian Hofhammer @ 2026-02-26 11:55 UTC (permalink / raw)
To: alex.bennee, pierrick.bouvier
Cc: richard.henderson, laurent, imp, berrange, qemu-devel
On 24/02/2026 16:53, 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.
>
> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
> ---
> include/plugins/qemu-plugin.h | 2 ++
> plugins/api.c | 16 ++++++++++++++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/include/plugins/qemu-plugin.h b/include/plugins/qemu-plugin.h
> index 04c884e82b..ae711758f1 100644
> --- a/include/plugins/qemu-plugin.h
> +++ b/include/plugins/qemu-plugin.h
> @@ -979,11 +979,13 @@ struct qemu_plugin_register;
> * writing value with qemu_plugin_write_register
> * @name: register name
> * @feature: optional feature descriptor, can be NULL
> + * @is_readonly: true if the register cannot be written via qemu_plugin_write_register
> */
> typedef struct {
> struct qemu_plugin_register *handle;
> const char *name;
> const char *feature;
> + bool is_readonly;
> } qemu_plugin_reg_descriptor;
>
> /**
> diff --git a/plugins/api.c b/plugins/api.c
> index ca3e93a194..b2c52d2a09 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 program counter
> +static const char rip_str[] = "rip"; // x86_64 specific name for program counter
> +static const char pswa_str[] = "pswa"; // s390x specific name for program counter
> +static const char iaoq_str[] = "iaoq"; // HP/PA specific name for program counter
> +static const char rpc_str[] = "rpc"; // microblaze specific name for program counter
> static GArray *create_register_handles(GArray *gdbstub_regs)
> {
> GArray *find_data = g_array_new(true, true,
> @@ -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);
> }
While extending the tests for my patches, I noticed that there are other
registers that are effectively read-only as well, such as the vg (vector
granule) register on aarch64. I wonder whether there should be a more
generic support in QEMU for this or whether this is something that
should be handled by GDB instead.
The difference to the pc in that case is that the pc architecturally is
read-write and it was just QEMU who silently swallowed the writes to it.
For the vg register in the example, as far as I understand correctly,
this is not an actual architectural register, but something that GDB
exposes as a read-only register for easier debugging, and QEMU therefore
also exposes this register. However, writes to it with
gdb_write_register and by proxy qemu_plugin_write_register are silently
ignored.
I'm not sure this is something that should be handled in this patch
series but I'm wondering whether this is something you already had on
your radar or whether this should be taken care of in a follow-up patch
series. I'd be happy to get your thoughts on this!
Best regards,
Florian
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 5/7] plugins: add read-only property for registers
2026-02-26 11:55 ` Florian Hofhammer
@ 2026-02-26 14:33 ` Alex Bennée
2026-02-26 19:43 ` Pierrick Bouvier
0 siblings, 1 reply; 46+ messages in thread
From: Alex Bennée @ 2026-02-26 14:33 UTC (permalink / raw)
To: Florian Hofhammer
Cc: pierrick.bouvier, richard.henderson, laurent, imp, berrange,
qemu-devel
Florian Hofhammer <florian.hofhammer@epfl.ch> writes:
> On 24/02/2026 16:53, 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.
>>
>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>> ---
>> include/plugins/qemu-plugin.h | 2 ++
>> plugins/api.c | 16 ++++++++++++++++
>> 2 files changed, 18 insertions(+)
>>
>> diff --git a/include/plugins/qemu-plugin.h b/include/plugins/qemu-plugin.h
>> index 04c884e82b..ae711758f1 100644
>> --- a/include/plugins/qemu-plugin.h
>> +++ b/include/plugins/qemu-plugin.h
>> @@ -979,11 +979,13 @@ struct qemu_plugin_register;
>> * writing value with qemu_plugin_write_register
>> * @name: register name
>> * @feature: optional feature descriptor, can be NULL
>> + * @is_readonly: true if the register cannot be written via qemu_plugin_write_register
>> */
>> typedef struct {
>> struct qemu_plugin_register *handle;
>> const char *name;
>> const char *feature;
>> + bool is_readonly;
>> } qemu_plugin_reg_descriptor;
>>
>> /**
>> diff --git a/plugins/api.c b/plugins/api.c
>> index ca3e93a194..b2c52d2a09 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 program counter
>> +static const char rip_str[] = "rip"; // x86_64 specific name for program counter
>> +static const char pswa_str[] = "pswa"; // s390x specific name for program counter
>> +static const char iaoq_str[] = "iaoq"; // HP/PA specific name for program counter
>> +static const char rpc_str[] = "rpc"; // microblaze specific name for program counter
>> static GArray *create_register_handles(GArray *gdbstub_regs)
>> {
>> GArray *find_data = g_array_new(true, true,
>> @@ -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);
>> }
>
> While extending the tests for my patches, I noticed that there are other
> registers that are effectively read-only as well, such as the vg (vector
> granule) register on aarch64. I wonder whether there should be a more
> generic support in QEMU for this or whether this is something that
> should be handled by GDB instead.
I was chatting to Pierrick about this yesterday. There are also
additional registers I'd like to make available as pure read-only and as
they go through the gdbstub machinery that seems the obvious place to
deal with them.
> The difference to the pc in that case is that the pc architecturally is
> read-write and it was just QEMU who silently swallowed the writes to it.
> For the vg register in the example, as far as I understand correctly,
> this is not an actual architectural register, but something that GDB
> exposes as a read-only register for easier debugging, and QEMU therefore
> also exposes this register. However, writes to it with
> gdb_write_register and by proxy qemu_plugin_write_register are silently
> ignored.
>
> I'm not sure this is something that should be handled in this patch
> series but I'm wondering whether this is something you already had on
> your radar or whether this should be taken care of in a follow-up patch
> series. I'd be happy to get your thoughts on this!
I think this is fine for now - for future enhancements we should extend
the gdb machinery and then reflect that in the exported plugin
registers.
>
> Best regards,
> Florian
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 5/7] plugins: add read-only property for registers
2026-02-26 14:33 ` Alex Bennée
@ 2026-02-26 19:43 ` Pierrick Bouvier
0 siblings, 0 replies; 46+ messages in thread
From: Pierrick Bouvier @ 2026-02-26 19:43 UTC (permalink / raw)
To: Alex Bennée, Florian Hofhammer
Cc: richard.henderson, laurent, imp, berrange, qemu-devel
On 2/26/26 6:33 AM, Alex Bennée wrote:
> Florian Hofhammer <florian.hofhammer@epfl.ch> writes:
>
>> On 24/02/2026 16:53, 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.
>>>
>>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>>> ---
>>> include/plugins/qemu-plugin.h | 2 ++
>>> plugins/api.c | 16 ++++++++++++++++
>>> 2 files changed, 18 insertions(+)
>>>
>>> diff --git a/include/plugins/qemu-plugin.h b/include/plugins/qemu-plugin.h
>>> index 04c884e82b..ae711758f1 100644
>>> --- a/include/plugins/qemu-plugin.h
>>> +++ b/include/plugins/qemu-plugin.h
>>> @@ -979,11 +979,13 @@ struct qemu_plugin_register;
>>> * writing value with qemu_plugin_write_register
>>> * @name: register name
>>> * @feature: optional feature descriptor, can be NULL
>>> + * @is_readonly: true if the register cannot be written via qemu_plugin_write_register
>>> */
>>> typedef struct {
>>> struct qemu_plugin_register *handle;
>>> const char *name;
>>> const char *feature;
>>> + bool is_readonly;
>>> } qemu_plugin_reg_descriptor;
>>>
>>> /**
>>> diff --git a/plugins/api.c b/plugins/api.c
>>> index ca3e93a194..b2c52d2a09 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 program counter
>>> +static const char rip_str[] = "rip"; // x86_64 specific name for program counter
>>> +static const char pswa_str[] = "pswa"; // s390x specific name for program counter
>>> +static const char iaoq_str[] = "iaoq"; // HP/PA specific name for program counter
>>> +static const char rpc_str[] = "rpc"; // microblaze specific name for program counter
>>> static GArray *create_register_handles(GArray *gdbstub_regs)
>>> {
>>> GArray *find_data = g_array_new(true, true,
>>> @@ -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);
>>> }
>>
>> While extending the tests for my patches, I noticed that there are other
>> registers that are effectively read-only as well, such as the vg (vector
>> granule) register on aarch64. I wonder whether there should be a more
>> generic support in QEMU for this or whether this is something that
>> should be handled by GDB instead.
>
> I was chatting to Pierrick about this yesterday. There are also
> additional registers I'd like to make available as pure read-only and as
> they go through the gdbstub machinery that seems the obvious place to
> deal with them.
>
In addition, it seems that the set of register who might be readonly
could be different between plugins and gdb.
For instance, it can be convenient to set pc by writing to register in
gdb, even though that's not something supported by plugins.
So I would *very* prudent to change gdbstub default as a side effect.
For now, and the for the plugins, the current scope is fine.
>> The difference to the pc in that case is that the pc architecturally is
>> read-write and it was just QEMU who silently swallowed the writes to it.
>> For the vg register in the example, as far as I understand correctly,
>> this is not an actual architectural register, but something that GDB
>> exposes as a read-only register for easier debugging, and QEMU therefore
>> also exposes this register. However, writes to it with
>> gdb_write_register and by proxy qemu_plugin_write_register are silently
>> ignored.
>>
>> I'm not sure this is something that should be handled in this patch
>> series but I'm wondering whether this is something you already had on
>> your radar or whether this should be taken care of in a follow-up patch
>> series. I'd be happy to get your thoughts on this!
>
> I think this is fine for now - for future enhancements we should extend
> the gdb machinery and then reflect that in the exported plugin
> registers.
>
>
>>
>> Best regards,
>> Florian
>
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v4 6/7] plugins: prohibit writing to read-only registers
2026-02-24 15:46 [PATCH v4 0/7] Enable PC diversion via the plugin API Florian Hofhammer
` (4 preceding siblings ...)
2026-02-24 15:53 ` [PATCH v4 5/7] plugins: add read-only property for registers Florian Hofhammer
@ 2026-02-24 15:57 ` Florian Hofhammer
2026-02-24 17:49 ` Alex Bennée
2026-02-24 15:58 ` [PATCH v4 7/7] tests/tcg/plugins: test register readonly feature Florian Hofhammer
2026-02-24 20:14 ` [PATCH v4 0/7] Enable PC diversion via the plugin API Pierrick Bouvier
7 siblings, 1 reply; 46+ messages in thread
From: Florian Hofhammer @ 2026-02-24 15:57 UTC (permalink / raw)
To: qemu-devel
Cc: alex.bennee, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
The opaque register handle encodes whether a register is read-only in
the lowest bit and prevents writing to the register via the plugin API
in this case.
Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
---
plugins/api.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/plugins/api.c b/plugins/api.c
index b2c52d2a09..3a8479ddce 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -424,6 +424,7 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
for (int i = 0; i < gdbstub_regs->len; i++) {
GDBRegDesc *grd = &g_array_index(gdbstub_regs, GDBRegDesc, i);
qemu_plugin_reg_descriptor desc;
+ gint plugin_ro_bit = 0;
/* skip "un-named" regs */
if (!grd->name) {
@@ -431,7 +432,6 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
}
/* Create a record for the plugin */
- desc.handle = GINT_TO_POINTER(grd->gdb_reg + 1);
desc.name = g_intern_string(grd->name);
desc.is_readonly = false;
if (g_strcmp0(desc.name, pc_str) == 0
@@ -442,7 +442,9 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
|| g_strcmp0(desc.name, rpc_str) == 0
) {
desc.is_readonly = true;
+ plugin_ro_bit = 1;
}
+ desc.handle = GINT_TO_POINTER((grd->gdb_reg << 1) | plugin_ro_bit);
desc.feature = g_intern_string(grd->feature_name);
g_array_append_val(find_data, desc);
}
@@ -467,7 +469,7 @@ bool qemu_plugin_read_register(struct qemu_plugin_register *reg,
return false;
}
- return (gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1) > 0);
+ return (gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) >> 1) > 0);
}
bool qemu_plugin_write_register(struct qemu_plugin_register *reg,
@@ -476,11 +478,12 @@ bool qemu_plugin_write_register(struct qemu_plugin_register *reg,
g_assert(current_cpu);
if (buf->len == 0 || (qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS &&
- qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS_PC)) {
+ qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS_PC)
+ || (GPOINTER_TO_INT(reg) & 1)) {
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] 46+ messages in thread* Re: [PATCH v4 6/7] plugins: prohibit writing to read-only registers
2026-02-24 15:57 ` [PATCH v4 6/7] plugins: prohibit writing to read-only registers Florian Hofhammer
@ 2026-02-24 17:49 ` Alex Bennée
2026-03-02 11:52 ` Florian Hofhammer
0 siblings, 1 reply; 46+ messages in thread
From: Alex Bennée @ 2026-02-24 17:49 UTC (permalink / raw)
To: Florian Hofhammer
Cc: qemu-devel, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
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>
> ---
> plugins/api.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/plugins/api.c b/plugins/api.c
> index b2c52d2a09..3a8479ddce 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -424,6 +424,7 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
> for (int i = 0; i < gdbstub_regs->len; i++) {
> GDBRegDesc *grd = &g_array_index(gdbstub_regs, GDBRegDesc, i);
> qemu_plugin_reg_descriptor desc;
> + gint plugin_ro_bit = 0;
>
> /* skip "un-named" regs */
> if (!grd->name) {
> @@ -431,7 +432,6 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
> }
>
> /* Create a record for the plugin */
> - desc.handle = GINT_TO_POINTER(grd->gdb_reg + 1);
> desc.name = g_intern_string(grd->name);
> desc.is_readonly = false;
> if (g_strcmp0(desc.name, pc_str) == 0
> @@ -442,7 +442,9 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
> || g_strcmp0(desc.name, rpc_str) == 0
> ) {
> desc.is_readonly = true;
> + plugin_ro_bit = 1;
> }
> + desc.handle = GINT_TO_POINTER((grd->gdb_reg << 1) | plugin_ro_bit);
> desc.feature = g_intern_string(grd->feature_name);
> g_array_append_val(find_data, desc);
> }
> @@ -467,7 +469,7 @@ bool qemu_plugin_read_register(struct qemu_plugin_register *reg,
> return false;
> }
>
> - return (gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1) > 0);
> + return (gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) >> 1) > 0);
> }
>
> bool qemu_plugin_write_register(struct qemu_plugin_register *reg,
> @@ -476,11 +478,12 @@ bool qemu_plugin_write_register(struct qemu_plugin_register *reg,
> g_assert(current_cpu);
>
> if (buf->len == 0 || (qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS &&
> - qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS_PC)) {
> + qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS_PC)
> + || (GPOINTER_TO_INT(reg) & 1)) {
Maybe this is better as:
g_assert(GPOINTER_TO_INT(reg) & 1 == 0);
if (buf->len == 0 || (qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS &&...
again the plugin is trying to do something it shouldn't.
> 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)
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 6/7] plugins: prohibit writing to read-only registers
2026-02-24 17:49 ` Alex Bennée
@ 2026-03-02 11:52 ` Florian Hofhammer
2026-03-02 13:03 ` Alex Bennée
0 siblings, 1 reply; 46+ messages in thread
From: Florian Hofhammer @ 2026-03-02 11:52 UTC (permalink / raw)
To: Alex Bennée
Cc: qemu-devel, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
On 24/02/2026 18:49, Alex Bennée wrote:
> 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>
>> ---
>> plugins/api.c | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/plugins/api.c b/plugins/api.c
>> index b2c52d2a09..3a8479ddce 100644
>> --- a/plugins/api.c
>> +++ b/plugins/api.c
>> @@ -424,6 +424,7 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
>> for (int i = 0; i < gdbstub_regs->len; i++) {
>> GDBRegDesc *grd = &g_array_index(gdbstub_regs, GDBRegDesc, i);
>> qemu_plugin_reg_descriptor desc;
>> + gint plugin_ro_bit = 0;
>>
>> /* skip "un-named" regs */
>> if (!grd->name) {
>> @@ -431,7 +432,6 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
>> }
>>
>> /* Create a record for the plugin */
>> - desc.handle = GINT_TO_POINTER(grd->gdb_reg + 1);
>> desc.name = g_intern_string(grd->name);
>> desc.is_readonly = false;
>> if (g_strcmp0(desc.name, pc_str) == 0
>> @@ -442,7 +442,9 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
>> || g_strcmp0(desc.name, rpc_str) == 0
>> ) {
>> desc.is_readonly = true;
>> + plugin_ro_bit = 1;
>> }
>> + desc.handle = GINT_TO_POINTER((grd->gdb_reg << 1) | plugin_ro_bit);
>> desc.feature = g_intern_string(grd->feature_name);
>> g_array_append_val(find_data, desc);
>> }
>> @@ -467,7 +469,7 @@ bool qemu_plugin_read_register(struct qemu_plugin_register *reg,
>> return false;
>> }
>>
>> - return (gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1) > 0);
>> + return (gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) >> 1) > 0);
>> }
>>
>> bool qemu_plugin_write_register(struct qemu_plugin_register *reg,
>> @@ -476,11 +478,12 @@ bool qemu_plugin_write_register(struct qemu_plugin_register *reg,
>> g_assert(current_cpu);
>>
>> if (buf->len == 0 || (qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS &&
>> - qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS_PC)) {
>> + qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS_PC)
>> + || (GPOINTER_TO_INT(reg) & 1)) {
>
> Maybe this is better as:
>
> g_assert(GPOINTER_TO_INT(reg) & 1 == 0);
>
> if (buf->len == 0 || (qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS &&...
>
> again the plugin is trying to do something it shouldn't.
As far as I can tell, there's currently no mechanism in the test setup
to check that a certain assertion is triggered. In the previous test, I
just checked whether the API returns false when I try to write to a
read-only register, but now I'd need to check in my test whether the
assert triggers.
Should I check that in a wrapper script, test only the read path, or
follow a completely different approach for this?
>
>> return false;
>> }
>>
>> - return (gdb_write_register(current_cpu, buf->data, GPOINTER_TO_INT(reg) - 1) > 0);
>> + return (gdb_write_register(current_cpu, buf->data, GPOINTER_TO_INT(reg) >> 1) > 0);
>> }
>>
>> void qemu_plugin_set_pc(uint64_t vaddr)
>
Best regards,
Florian
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 6/7] plugins: prohibit writing to read-only registers
2026-03-02 11:52 ` Florian Hofhammer
@ 2026-03-02 13:03 ` Alex Bennée
2026-03-02 13:06 ` Florian Hofhammer
0 siblings, 1 reply; 46+ messages in thread
From: Alex Bennée @ 2026-03-02 13:03 UTC (permalink / raw)
To: Florian Hofhammer
Cc: qemu-devel, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
Florian Hofhammer <florian.hofhammer@epfl.ch> writes:
> On 24/02/2026 18:49, Alex Bennée wrote:
>> 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>
>>> ---
>>> plugins/api.c | 11 +++++++----
>>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/plugins/api.c b/plugins/api.c
>>> index b2c52d2a09..3a8479ddce 100644
>>> --- a/plugins/api.c
>>> +++ b/plugins/api.c
>>> @@ -424,6 +424,7 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
>>> for (int i = 0; i < gdbstub_regs->len; i++) {
>>> GDBRegDesc *grd = &g_array_index(gdbstub_regs, GDBRegDesc, i);
>>> qemu_plugin_reg_descriptor desc;
>>> + gint plugin_ro_bit = 0;
>>>
>>> /* skip "un-named" regs */
>>> if (!grd->name) {
>>> @@ -431,7 +432,6 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
>>> }
>>>
>>> /* Create a record for the plugin */
>>> - desc.handle = GINT_TO_POINTER(grd->gdb_reg + 1);
>>> desc.name = g_intern_string(grd->name);
>>> desc.is_readonly = false;
>>> if (g_strcmp0(desc.name, pc_str) == 0
>>> @@ -442,7 +442,9 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
>>> || g_strcmp0(desc.name, rpc_str) == 0
>>> ) {
>>> desc.is_readonly = true;
>>> + plugin_ro_bit = 1;
>>> }
>>> + desc.handle = GINT_TO_POINTER((grd->gdb_reg << 1) | plugin_ro_bit);
>>> desc.feature = g_intern_string(grd->feature_name);
>>> g_array_append_val(find_data, desc);
>>> }
>>> @@ -467,7 +469,7 @@ bool qemu_plugin_read_register(struct qemu_plugin_register *reg,
>>> return false;
>>> }
>>>
>>> - return (gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1) > 0);
>>> + return (gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) >> 1) > 0);
>>> }
>>>
>>> bool qemu_plugin_write_register(struct qemu_plugin_register *reg,
>>> @@ -476,11 +478,12 @@ bool qemu_plugin_write_register(struct qemu_plugin_register *reg,
>>> g_assert(current_cpu);
>>>
>>> if (buf->len == 0 || (qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS &&
>>> - qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS_PC)) {
>>> + qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS_PC)
>>> + || (GPOINTER_TO_INT(reg) & 1)) {
>>
>> Maybe this is better as:
>>
>> g_assert(GPOINTER_TO_INT(reg) & 1 == 0);
>>
>> if (buf->len == 0 || (qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS &&...
>>
>> again the plugin is trying to do something it shouldn't.
>
> As far as I can tell, there's currently no mechanism in the test setup
> to check that a certain assertion is triggered. In the previous test, I
> just checked whether the API returns false when I try to write to a
> read-only register, but now I'd need to check in my test whether the
> assert triggers.
Ahh I see - I was going to say we don't really need to test for abuse of
the API triggering asserts but you next test does indeed do that. I
think we can live without explicitly adding a test case for attempts to
write to read only registers.
>
> Should I check that in a wrapper script, test only the read path, or
> follow a completely different approach for this?
>
>>
>>> return false;
>>> }
>>>
>>> - return (gdb_write_register(current_cpu, buf->data, GPOINTER_TO_INT(reg) - 1) > 0);
>>> + return (gdb_write_register(current_cpu, buf->data, GPOINTER_TO_INT(reg) >> 1) > 0);
>>> }
>>>
>>> void qemu_plugin_set_pc(uint64_t vaddr)
>>
>
> Best regards,
> Florian
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 6/7] plugins: prohibit writing to read-only registers
2026-03-02 13:03 ` Alex Bennée
@ 2026-03-02 13:06 ` Florian Hofhammer
0 siblings, 0 replies; 46+ messages in thread
From: Florian Hofhammer @ 2026-03-02 13:06 UTC (permalink / raw)
To: Alex Bennée
Cc: qemu-devel, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
On 02/03/2026 14:03, Alex Bennée wrote:
> Florian Hofhammer <florian.hofhammer@epfl.ch> writes:
>
>> On 24/02/2026 18:49, Alex Bennée wrote:
>>> 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>
>>>> ---
>>>> plugins/api.c | 11 +++++++----
>>>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/plugins/api.c b/plugins/api.c
>>>> index b2c52d2a09..3a8479ddce 100644
>>>> --- a/plugins/api.c
>>>> +++ b/plugins/api.c
>>>> @@ -424,6 +424,7 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
>>>> for (int i = 0; i < gdbstub_regs->len; i++) {
>>>> GDBRegDesc *grd = &g_array_index(gdbstub_regs, GDBRegDesc, i);
>>>> qemu_plugin_reg_descriptor desc;
>>>> + gint plugin_ro_bit = 0;
>>>>
>>>> /* skip "un-named" regs */
>>>> if (!grd->name) {
>>>> @@ -431,7 +432,6 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
>>>> }
>>>>
>>>> /* Create a record for the plugin */
>>>> - desc.handle = GINT_TO_POINTER(grd->gdb_reg + 1);
>>>> desc.name = g_intern_string(grd->name);
>>>> desc.is_readonly = false;
>>>> if (g_strcmp0(desc.name, pc_str) == 0
>>>> @@ -442,7 +442,9 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
>>>> || g_strcmp0(desc.name, rpc_str) == 0
>>>> ) {
>>>> desc.is_readonly = true;
>>>> + plugin_ro_bit = 1;
>>>> }
>>>> + desc.handle = GINT_TO_POINTER((grd->gdb_reg << 1) | plugin_ro_bit);
>>>> desc.feature = g_intern_string(grd->feature_name);
>>>> g_array_append_val(find_data, desc);
>>>> }
>>>> @@ -467,7 +469,7 @@ bool qemu_plugin_read_register(struct qemu_plugin_register *reg,
>>>> return false;
>>>> }
>>>>
>>>> - return (gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1) > 0);
>>>> + return (gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) >> 1) > 0);
>>>> }
>>>>
>>>> bool qemu_plugin_write_register(struct qemu_plugin_register *reg,
>>>> @@ -476,11 +478,12 @@ bool qemu_plugin_write_register(struct qemu_plugin_register *reg,
>>>> g_assert(current_cpu);
>>>>
>>>> if (buf->len == 0 || (qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS &&
>>>> - qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS_PC)) {
>>>> + qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS_PC)
>>>> + || (GPOINTER_TO_INT(reg) & 1)) {
>>>
>>> Maybe this is better as:
>>>
>>> g_assert(GPOINTER_TO_INT(reg) & 1 == 0);
>>>
>>> if (buf->len == 0 || (qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS &&...
>>>
>>> again the plugin is trying to do something it shouldn't.
>>
>> As far as I can tell, there's currently no mechanism in the test setup
>> to check that a certain assertion is triggered. In the previous test, I
>> just checked whether the API returns false when I try to write to a
>> read-only register, but now I'd need to check in my test whether the
>> assert triggers.
>
> Ahh I see - I was going to say we don't really need to test for abuse of
> the API triggering asserts but you next test does indeed do that. I
> think we can live without explicitly adding a test case for attempts to
> write to read only registers.
Ack, thanks for the quick reply!
>
>>
>> Should I check that in a wrapper script, test only the read path, or
>> follow a completely different approach for this?
>>
>>>
>>>> return false;
>>>> }
>>>>
>>>> - return (gdb_write_register(current_cpu, buf->data, GPOINTER_TO_INT(reg) - 1) > 0);
>>>> + return (gdb_write_register(current_cpu, buf->data, GPOINTER_TO_INT(reg) >> 1) > 0);
>>>> }
>>>>
>>>> void qemu_plugin_set_pc(uint64_t vaddr)
>>>
>>
>> Best regards,
>> Florian
>
^ permalink raw reply [flat|nested] 46+ messages in thread
* [PATCH v4 7/7] tests/tcg/plugins: test register readonly feature
2026-02-24 15:46 [PATCH v4 0/7] Enable PC diversion via the plugin API Florian Hofhammer
` (5 preceding siblings ...)
2026-02-24 15:57 ` [PATCH v4 6/7] plugins: prohibit writing to read-only registers Florian Hofhammer
@ 2026-02-24 15:58 ` Florian Hofhammer
2026-02-24 20:17 ` Pierrick Bouvier
2026-02-25 9:24 ` Alex Bennée
2026-02-24 20:14 ` [PATCH v4 0/7] Enable PC diversion via the plugin API Pierrick Bouvier
7 siblings, 2 replies; 46+ messages in thread
From: Florian Hofhammer @ 2026-02-24 15:58 UTC (permalink / raw)
To: qemu-devel
Cc: alex.bennee, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
This commit tests the readonly feature for registers exposed via the
plugin API introduced earlier in the patch series.
Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
---
tests/tcg/plugins/meson.build | 1 +
tests/tcg/plugins/registers.c | 68 +++++++++++++++++++++++++++++++++++
2 files changed, 69 insertions(+)
create mode 100644 tests/tcg/plugins/registers.c
diff --git a/tests/tcg/plugins/meson.build b/tests/tcg/plugins/meson.build
index c5e49753fd..5005aa8dc9 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',
'syscall.c',
]
diff --git a/tests/tcg/plugins/registers.c b/tests/tcg/plugins/registers.c
new file mode 100644
index 0000000000..a7bd692225
--- /dev/null
+++ b/tests/tcg/plugins/registers.c
@@ -0,0 +1,68 @@
+/*
+ * Copyright (C) 2026, Florian Hofhammer <florian.hofhammer@epfl.ch>
+ *
+ * License: GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "glib.h"
+#include <inttypes.h>
+#include <assert.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <stdio.h>
+
+#include <qemu-plugin.h>
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
+/*
+ * This callback is called when a vCPU is initialized. It tests whether we
+ * successfully read from a register and write value back to it. It also tests
+ * that read-only registers cannot be written to, i.e., we can read a read-only
+ * register but writing to it fails.
+ */
+static void vcpu_init_cb(qemu_plugin_id_t id, unsigned int vcpu_index)
+{
+ g_autoptr(GArray) regs = qemu_plugin_get_registers();
+ g_autoptr(GByteArray) buf = g_byte_array_sized_new(0);
+ bool success = false;
+
+ /* Make sure we can read and write an arbitrary register */
+ qemu_plugin_reg_descriptor *reg_desc = &g_array_index(regs,
+ qemu_plugin_reg_descriptor, 0);
+ g_assert(reg_desc->is_readonly == false);
+ success = qemu_plugin_read_register(reg_desc->handle, buf);
+ g_assert(success);
+ success = qemu_plugin_write_register(reg_desc->handle, buf);
+ g_assert(success);
+
+ /*
+ * Reset the buffer and find a read-only register. On each architecture, at
+ * least the PC should be read-only because it's only supposed to be
+ * modified via the qemu_plugin_set_pc() function.
+ */
+ g_byte_array_set_size(buf, 0);
+ for (size_t i = 0; i < regs->len; i++) {
+ reg_desc = &g_array_index(regs, qemu_plugin_reg_descriptor, i);
+ if (reg_desc->is_readonly) {
+ success = qemu_plugin_read_register(reg_desc->handle, buf);
+ g_assert(success);
+ break;
+ } else {
+ reg_desc = NULL;
+ }
+ }
+ /* Make sure there is a read-only register and we cannot write to it */
+ g_assert(reg_desc != NULL);
+ success = qemu_plugin_write_register(reg_desc->handle, buf);
+ g_assert(!success);
+}
+
+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] 46+ messages in thread* Re: [PATCH v4 7/7] tests/tcg/plugins: test register readonly feature
2026-02-24 15:58 ` [PATCH v4 7/7] tests/tcg/plugins: test register readonly feature Florian Hofhammer
@ 2026-02-24 20:17 ` Pierrick Bouvier
2026-02-25 9:24 ` Alex Bennée
1 sibling, 0 replies; 46+ messages in thread
From: Pierrick Bouvier @ 2026-02-24 20:17 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: alex.bennee, richard.henderson, laurent, imp, berrange
On 2/24/26 7:58 AM, Florian Hofhammer wrote:
> This commit tests the readonly feature for registers exposed via the
> plugin API introduced earlier in the patch series.
>
> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
> ---
> tests/tcg/plugins/meson.build | 1 +
> tests/tcg/plugins/registers.c | 68 +++++++++++++++++++++++++++++++++++
> 2 files changed, 69 insertions(+)
> create mode 100644 tests/tcg/plugins/registers.c
>
> diff --git a/tests/tcg/plugins/meson.build b/tests/tcg/plugins/meson.build
> index c5e49753fd..5005aa8dc9 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',
> 'syscall.c',
> ]
> diff --git a/tests/tcg/plugins/registers.c b/tests/tcg/plugins/registers.c
> new file mode 100644
> index 0000000000..a7bd692225
> --- /dev/null
> +++ b/tests/tcg/plugins/registers.c
> @@ -0,0 +1,68 @@
> +/*
> + * Copyright (C) 2026, Florian Hofhammer <florian.hofhammer@epfl.ch>
> + *
> + * License: GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +#include "glib.h"
> +#include <inttypes.h>
> +#include <assert.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <stdio.h>
> +
> +#include <qemu-plugin.h>
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
> +
> +/*
> + * This callback is called when a vCPU is initialized. It tests whether we
> + * successfully read from a register and write value back to it. It also tests
> + * that read-only registers cannot be written to, i.e., we can read a read-only
> + * register but writing to it fails.
> + */
> +static void vcpu_init_cb(qemu_plugin_id_t id, unsigned int vcpu_index)
> +{
> + g_autoptr(GArray) regs = qemu_plugin_get_registers();
> + g_autoptr(GByteArray) buf = g_byte_array_sized_new(0);
> + bool success = false;
> +
> + /* Make sure we can read and write an arbitrary register */
> + qemu_plugin_reg_descriptor *reg_desc = &g_array_index(regs,
> + qemu_plugin_reg_descriptor, 0);
> + g_assert(reg_desc->is_readonly == false);
> + success = qemu_plugin_read_register(reg_desc->handle, buf);
> + g_assert(success);
> + success = qemu_plugin_write_register(reg_desc->handle, buf);
> + g_assert(success);
> +
> + /*
> + * Reset the buffer and find a read-only register. On each architecture, at
> + * least the PC should be read-only because it's only supposed to be
> + * modified via the qemu_plugin_set_pc() function.
> + */
> + g_byte_array_set_size(buf, 0);
> + for (size_t i = 0; i < regs->len; i++) {
> + reg_desc = &g_array_index(regs, qemu_plugin_reg_descriptor, i);
> + if (reg_desc->is_readonly) {
> + success = qemu_plugin_read_register(reg_desc->handle, buf);
> + g_assert(success);
> + break;
> + } else {
> + reg_desc = NULL;
> + }
> + }
> + /* Make sure there is a read-only register and we cannot write to it */
> + g_assert(reg_desc != NULL);
> + success = qemu_plugin_write_register(reg_desc->handle, buf);
> + g_assert(!success);
> +}
> +
> +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;
> +}
Looks great, and nice to have a test for this.
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 46+ messages in thread* Re: [PATCH v4 7/7] tests/tcg/plugins: test register readonly feature
2026-02-24 15:58 ` [PATCH v4 7/7] tests/tcg/plugins: test register readonly feature Florian Hofhammer
2026-02-24 20:17 ` Pierrick Bouvier
@ 2026-02-25 9:24 ` Alex Bennée
1 sibling, 0 replies; 46+ messages in thread
From: Alex Bennée @ 2026-02-25 9:24 UTC (permalink / raw)
To: Florian Hofhammer
Cc: qemu-devel, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
Florian Hofhammer <florian.hofhammer@epfl.ch> writes:
> This commit tests the readonly feature for registers exposed via the
> plugin API introduced earlier in the patch series.
>
> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
Acked-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [PATCH v4 0/7] Enable PC diversion via the plugin API
2026-02-24 15:46 [PATCH v4 0/7] Enable PC diversion via the plugin API Florian Hofhammer
` (6 preceding siblings ...)
2026-02-24 15:58 ` [PATCH v4 7/7] tests/tcg/plugins: test register readonly feature Florian Hofhammer
@ 2026-02-24 20:14 ` Pierrick Bouvier
7 siblings, 0 replies; 46+ messages in thread
From: Pierrick Bouvier @ 2026-02-24 20:14 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: alex.bennee, richard.henderson, laurent, imp, berrange
On 2/24/26 7:46 AM, Florian Hofhammer wrote:
> Hi,
>
> This patch series builds on top of the discussion from the thread at
> https://lore.kernel.org/qemu-devel/e9bcd7c7-2d67-469e-b2f3-d1a68e456b2b@epfl.ch/
> and adds a plugin API function to set the program counter of the guest,
> as just writing to it via qemu_plugin_write_register() has no direct
> effect.
>
> Based on the discussion in the above thread, the series also introduces
> a means to declare registers as read-only from the plugin side, which
> prevents plugins from writing to them via qemu_plugin_write_register().
> This for now is only applied to the PC, and finding the PC register is
> done via some rather hacky strcmp()s. In the above thread, we also
> discussed encoding the read-only property in a custom attribute in the
> GDB XMLs, but that would (1) make syncing with GDB harder, (2) not cover
> all architectures, as there's not an XML description of all
> architectures available in the gdb-xml/ directory, and (3) require quite
> some changes to the whole GDB infrastructure in gdbstub/ to even encode
> the attribute in the correct structs and pass them on over the different
> layers up into the plugin API.
>
> This version v4 of the patch series is more about small refactorings and
> cleanups than changes in functionality.
>
> Best regards,
> Florian
>
> Changes:
> 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 test 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 readonly feature
>
> include/plugins/qemu-plugin.h | 18 +++++
> linux-user/aarch64/cpu_loop.c | 2 +-
> linux-user/alpha/cpu_loop.c | 2 +-
> linux-user/arm/cpu_loop.c | 2 +-
> linux-user/hexagon/cpu_loop.c | 2 +-
> linux-user/hppa/cpu_loop.c | 4 ++
> linux-user/i386/cpu_loop.c | 8 ++-
> linux-user/include/special-errno.h | 8 +++
> linux-user/loongarch64/cpu_loop.c | 5 +-
> linux-user/m68k/cpu_loop.c | 2 +-
> linux-user/microblaze/cpu_loop.c | 2 +-
> linux-user/mips/cpu_loop.c | 5 +-
> linux-user/or1k/cpu_loop.c | 2 +-
> linux-user/ppc/cpu_loop.c | 6 +-
> linux-user/riscv/cpu_loop.c | 2 +-
> linux-user/s390x/cpu_loop.c | 2 +-
> linux-user/sh4/cpu_loop.c | 2 +-
> linux-user/sparc/cpu_loop.c | 4 +-
> linux-user/syscall.c | 16 +++++
> linux-user/xtensa/cpu_loop.c | 3 +
> plugins/api.c | 43 ++++++++++--
> plugins/core.c | 29 ++++----
> tests/tcg/arm/Makefile.target | 6 ++
> tests/tcg/hexagon/Makefile.target | 7 ++
> tests/tcg/mips/Makefile.target | 6 +-
> tests/tcg/mips64/Makefile.target | 15 ++++
> tests/tcg/mips64el/Makefile.target | 15 ++++
> tests/tcg/mipsel/Makefile.target | 15 ++++
> tests/tcg/multiarch/Makefile.target | 22 +++++-
> .../{ => plugin}/check-plugin-output.sh | 0
> .../{ => plugin}/test-plugin-mem-access.c | 0
> .../plugin/test-plugin-skip-syscalls.c | 26 +++++++
> tests/tcg/plugins/meson.build | 1 +
> tests/tcg/plugins/registers.c | 68 +++++++++++++++++++
> tests/tcg/plugins/syscall.c | 6 ++
> tests/tcg/sparc64/Makefile.target | 16 +++++
> 36 files changed, 331 insertions(+), 41 deletions(-)
> create mode 100644 tests/tcg/mips64/Makefile.target
> create mode 100644 tests/tcg/mips64el/Makefile.target
> create mode 100644 tests/tcg/mipsel/Makefile.target
> rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
> rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
> create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
> create mode 100644 tests/tcg/plugins/registers.c
> create mode 100644 tests/tcg/sparc64/Makefile.target
>
>
> base-commit: afe653676dc6dfd49f0390239ff90b2f0052c2b8
A few small nits reported by checkpatch.
$ ./scripts/checkpatch.pl $(git merge-base upstream/master HEAD)..HEAD
1/8 Checking commit 23b2901d559b (plugins: add flag to specify whether
PC is rw)
2/8 Checking commit cf46ff49010c (linux-user: make syscall emulation
interruptible)
WARNING: Block comments use a leading /* on a separate line
#217: FILE: linux-user/mips/cpu_loop.c:144:
+ /* Returning from a successful sigreturn syscall or from
WARNING: Block comments use * on subsequent lines
#218: FILE: linux-user/mips/cpu_loop.c:145:
+ /* Returning from a successful sigreturn syscall or from
+ control flow diversion in a plugin callback.
WARNING: Block comments use a leading /* on a separate line
#247: FILE: linux-user/ppc/cpu_loop.c:345:
+ /* Returning from a successful sigreturn syscall or from
WARNING: Block comments use * on subsequent lines
#248: FILE: linux-user/ppc/cpu_loop.c:346:
+ /* Returning from a successful sigreturn syscall or from
+ control flow diversion in a plugin callback.
3/8 Checking commit 2bb085851ee5 (plugins: add PC diversion API function)
4/8 Checking commit 3001ad67e7e5 (tests/tcg: add test for
qemu_plugin_set_pc API)
ERROR: New file 'tests/tcg/mips64/Makefile.target' requires
'SPDX-License-Identifier'
ERROR: New file 'tests/tcg/mips64el/Makefile.target' requires
'SPDX-License-Identifier'
ERROR: New file 'tests/tcg/mipsel/Makefile.target' requires
'SPDX-License-Identifier'
ERROR: open brace '{' following function declarations go on the next line
#216: FILE: tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c:15:
+void exit_success(void) {
ERROR: open brace '{' following function declarations go on the next line
#220: FILE: tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c:19:
+int main(int argc, char *argv[]) {
ERROR: New file 'tests/tcg/sparc64/Makefile.target' requires
'SPDX-License-Identifier'
5/8 Checking commit c85ae80717dc (plugins: add read-only property for
registers)
ERROR: do not use C99 // comments
#45: FILE: plugins/api.c:413:
+static const char pc_str[] = "pc"; // generic name for program counter
ERROR: do not use C99 // comments
#46: FILE: plugins/api.c:414:
+static const char eip_str[] = "eip"; // x86 specific name for program
counter
ERROR: do not use C99 // comments
#47: FILE: plugins/api.c:415:
+static const char rip_str[] = "rip"; // x86_64 specific name for
program counter
ERROR: do not use C99 // comments
#48: FILE: plugins/api.c:416:
+static const char pswa_str[] = "pswa"; // s390x specific name for
program counter
ERROR: do not use C99 // comments
#49: FILE: plugins/api.c:417:
+static const char iaoq_str[] = "iaoq"; // HP/PA specific name for
program counter
ERROR: do not use C99 // comments
#50: FILE: plugins/api.c:418:
+static const char rpc_str[] = "rpc"; // microblaze specific name for
program counter
6/8 Checking commit 156dc74ab9a5 (plugins: prohibit writing to read-only
registers)
7/8 Checking commit dba748c1e46c (tests/tcg/plugins: test register
readonly feature)
ERROR: New file 'tests/tcg/plugins/registers.c' requires
'SPDX-License-Identifier'
ERROR: New file 'tests/tcg/plugins/registers.c' must not have license
boilerplate header text, only the SPDX-License-Identifier, unless this
file was copied from existing code already having such text.
Regards,
Pierrick
^ permalink raw reply [flat|nested] 46+ messages in thread