Julian Ganz <[email protected]> writes:
> We recently introduced plugin API for the registration of callbacks for
> discontinuity events, specifically for interrupts, exceptions and host
> call events. The callback receives various bits of information,
> including the VCPU index and PCs.
>
> This change introduces a test plugin asserting the correctness of that
> behaviour in cases where this is possible with reasonable effort.
>
> Reviewed-by: Pierrick Bouvier <[email protected]>
> Signed-off-by: Julian Ganz <[email protected]>
> ---
> tests/tcg/plugins/discons.c | 210 ++++++++++++++++++++++++++++++++++
> tests/tcg/plugins/meson.build | 2 +-
> 2 files changed, 211 insertions(+), 1 deletion(-)
> create mode 100644 tests/tcg/plugins/discons.c
>
> diff --git a/tests/tcg/plugins/discons.c b/tests/tcg/plugins/discons.c
> new file mode 100644
> index 0000000000..f185e3948b
> --- /dev/null
> +++ b/tests/tcg/plugins/discons.c
> @@ -0,0 +1,210 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + * Copyright (C) 2025, Julian Ganz <[email protected]>
> + *
> + * This plugin exercises the discontinuity plugin API and asserts some
> + * of its behaviour regarding reported program counters.
> + */
> +#include <stdio.h>
> +
> +#include <qemu-plugin.h>
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
> +
> +struct cpu_state {
> + uint64_t last_pc;
> + uint64_t from_pc;
> + uint64_t next_pc;
> + uint64_t has_from;
> + bool has_next;
> + enum qemu_plugin_discon_type next_type;
> +};
> +
> +struct insn_data {
> + uint64_t addr;
> + uint64_t next_pc;
> + bool next_valid;
> +};
> +
> +static struct qemu_plugin_scoreboard *states;
> +
> +static qemu_plugin_u64 last_pc;
> +static qemu_plugin_u64 from_pc;
> +static qemu_plugin_u64 has_from;
> +
> +static bool abort_on_mismatch;
> +static bool trace_all_insns;
> +
> +static bool addr_eq(uint64_t a, uint64_t b)
> +{
> + if (a == b) {
> + return true;
> + }
> +
> + uint64_t a_hw;
> + uint64_t b_hw;
> + if (!qemu_plugin_translate_vaddr(a, &a_hw) ||
> + !qemu_plugin_translate_vaddr(b, &b_hw))
> + {
> + return false;
> + }
> +
> + return a_hw == b_hw;
> +}
> +
> +static void report_mismatch(const char *pc_name, unsigned int vcpu_index,
> + enum qemu_plugin_discon_type type, uint64_t last,
> + uint64_t expected, uint64_t encountered)
> +{
> + GString *report;
This could be:
g_autoptr(GString) buf = g_string_new(NULL);
> + const char *discon_type_name = "unknown";
> +
> + if (addr_eq(expected, encountered)) {
> + return;
> + }
> +
> + switch (type) {
> + case QEMU_PLUGIN_DISCON_INTERRUPT:
> + discon_type_name = "interrupt";
> + break;
> + case QEMU_PLUGIN_DISCON_EXCEPTION:
> + discon_type_name = "exception";
> + break;
> + case QEMU_PLUGIN_DISCON_HOSTCALL:
> + discon_type_name = "hostcall";
> + break;
> + default:
> + break;
> + }
> +
> + report = g_string_new(NULL);
> + g_string_append_printf(report,
> + "Discon %s PC mismatch on VCPU %d\nExpected:
> %"
> + PRIx64"\nEncountered: %"PRIx64"\nExecuted Last:
> %"
> + PRIx64"\nEvent type: %s\n",
> + pc_name, vcpu_index, expected, encountered, last,
> + discon_type_name);
> + qemu_plugin_outs(report->str);
I think we might want to flush here because
> + if (abort_on_mismatch) {
> + g_abort();
> + }
This is firing on:
🕙17:35:50 alex@draig:tests/tcg/i386-linux-user on review/tcg-discon-v6
[$!?]
➜ make run-plugin-catch-syscalls-with-libdiscons.so V=1
timeout -s KILL --foreground 120 env
QEMU=/home/alex/lsrc/qemu.git/builds/sanitisers/qemu-i386
/home/alex/lsrc/qemu.git/builds/sanitisers/qemu-i386 -plugin
../plugins/libdiscons.so -d plugin -D catch-syscalls-with-libdiscons.so.pout
catch-syscalls > run-plugin-catch-syscalls-with-libdiscons.so.out
Aborted
make: *** [Makefile:226: run-plugin-catch-syscalls-with-libdiscons.so] Error
134
🕙17:35:52 alex@draig:tests/tcg/i386-linux-user on review/tcg-discon-v6
[$!?] [🔴 USAGE]
✗
although it never gets to the point of reporting what failed:
Thread 1 "qemu-i386" hit Breakpoint 1, __GI_abort () at ./stdlib/abort.c:72
warning: 72 ./stdlib/abort.c: No such file or directory
(gdb) bt
#0 __GI_abort () at ./stdlib/abort.c:72
#1 0x00007ffff630874d in report_mismatch (pc_name=0x7ffff630a220 "target",
vcpu_index=0, type=QEMU_PLUGIN_DISCON_EXCEPTION, last=134574955,
expected=134574953,
encountered=134574955) at ../../tests/tcg/plugins/discons.c:89
#2 0x00007ffff6308c0d in insn_exec (vcpu_index=0, userdata=0x0) at
../../tests/tcg/plugins/discons.c:132
#3 0x00007fffea431114 in code_gen_buffer ()
#4 0x000055555577b0a6 in cpu_tb_exec (cpu=0x529000005200, itb=0x7fffea431000
<code_gen_buffer+200659>, tb_exit=0x7ffff49c9530) at
../../accel/tcg/cpu-exec.c:438
#5 0x000055555577c92f in cpu_loop_exec_tb (cpu=0x529000005200,
tb=0x7fffea431000 <code_gen_buffer+200659>, pc=134574955,
last_tb=0x7ffff49c9540, tb_exit=0x7ffff49c9530)
at ../../accel/tcg/cpu-exec.c:871
#6 0x000055555577d151 in cpu_exec_loop (cpu=0x529000005200,
sc=0x7ffff483a740) at ../../accel/tcg/cpu-exec.c:981
#7 0x000055555577d2fe in cpu_exec_setjmp (cpu=0x529000005200,
sc=0x7ffff483a740) at ../../accel/tcg/cpu-exec.c:998
#8 0x000055555577d4c8 in cpu_exec (cpu=0x529000005200) at
../../accel/tcg/cpu-exec.c:1024
#9 0x00005555557bfc83 in cpu_loop (env=0x529000007dd0) at
../../linux-user/i386/cpu_loop.c:215
#10 0x00005555558ee3e1 in main (argc=4, argv=0x7fffffffe688,
envp=0x7fffffffe6b0) at ../../linux-user/main.c:1038
(gdb) f 1
#1 0x00007ffff630874d in report_mismatch (pc_name=0x7ffff630a220 "target",
vcpu_index=0, type=QEMU_PLUGIN_DISCON_EXCEPTION, last=134574955,
expected=134574953,
encountered=134574955) at ../../tests/tcg/plugins/discons.c:89
89 g_abort();
(gdb) p report
$1 = (GString *) 0x50300002bf00
(gdb) p report->Str
There is no member named Str.
(gdb) p report->str
$2 = (gchar *) 0x51100001fbc0 "Discon target PC mismatch on VCPU 0\nExpected:
8057369\nEncountered: 805736b\nExecuted Last: 805736b\nEvent type:
exception\n"
(gdb)
I think this is where it is going wrong:
IN: _dl_early_allocate
0x0805736b: 89 c2 movl %eax, %edx
0x0805736d: 8d 1c 28 leal (%eax, %ebp), %ebx
0x08057370: 89 c8 movl %ecx, %eax
0x08057372: cd 80 int $0x80
> + g_string_free(report, true);
so we could drop this... or..
> +}
> +
> +static void vcpu_discon(qemu_plugin_id_t id, unsigned int vcpu_index,
> + enum qemu_plugin_discon_type type, uint64_t from_pc,
> + uint64_t to_pc)
> +{
> + struct cpu_state *state = qemu_plugin_scoreboard_find(states,
> vcpu_index);
> +
> + if (type == QEMU_PLUGIN_DISCON_EXCEPTION &&
> + addr_eq(state->last_pc, from_pc))
> + {
> + /*
> + * For some types of exceptions, insn_exec will be called for the
> + * instruction that caused the exception. This is valid behaviour and
> + * does not need to be reported.
> + */
> + } else if (state->has_next) {
> + /*
> + * We may encounter discontinuity chains without any instructions
> + * being executed in between.
> + */
> + report_mismatch("source", vcpu_index, type, state->last_pc,
> + state->next_pc, from_pc);
> + } else if (state->has_from) {
> + report_mismatch("source", vcpu_index, type, state->last_pc,
> + state->from_pc, from_pc);
> + }
> +
> + state->has_from = false;
> +
> + state->next_pc = to_pc;
> + state->next_type = type;
> + state->has_next = true;
> +}
> +
> +static void insn_exec(unsigned int vcpu_index, void *userdata)
> +{
> + struct cpu_state *state = qemu_plugin_scoreboard_find(states,
> vcpu_index);
> +
> + if (state->has_next) {
> + report_mismatch("target", vcpu_index, state->next_type,
> state->last_pc,
> + state->next_pc, state->last_pc);
> + state->has_next = false;
> + }
> +
> + if (trace_all_insns) {
> + g_autoptr(GString) report = g_string_new(NULL);
> + g_string_append_printf(report, "Exec insn at %"PRIx64" on VCPU %d\n",
> + state->last_pc, vcpu_index);
> + qemu_plugin_outs(report->str);
> + }
> +}
> +
> +static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
> +{
> + size_t n_insns = qemu_plugin_tb_n_insns(tb);
> + for (size_t i = 0; i < n_insns; i++) {
> + struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
> + uint64_t pc = qemu_plugin_insn_vaddr(insn);
> + uint64_t next_pc = pc + qemu_plugin_insn_size(insn);
> + uint64_t has_next = (i + 1) < n_insns;
> +
> + qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(insn,
> +
> QEMU_PLUGIN_INLINE_STORE_U64,
> + last_pc, pc);
> + qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(insn,
> +
> QEMU_PLUGIN_INLINE_STORE_U64,
> + from_pc,
> next_pc);
> + qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(insn,
> +
> QEMU_PLUGIN_INLINE_STORE_U64,
> + has_from,
> has_next);
> + qemu_plugin_register_vcpu_insn_exec_cb(insn, insn_exec,
> + QEMU_PLUGIN_CB_NO_REGS, NULL);
> + }
> +}
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> + const qemu_info_t *info,
> + int argc, char **argv)
> +{
> + /* Set defaults */
> + abort_on_mismatch = true;
> + trace_all_insns = false;
> +
> + for (int i = 0; i < argc; i++) {
> + char *opt = argv[i];
> + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
> + if (g_strcmp0(tokens[0], "abort") == 0) {
> + if (!qemu_plugin_bool_parse(tokens[0], tokens[1],
> + &abort_on_mismatch)) {
> + fprintf(stderr, "boolean argument parsing failed: %s\n",
> opt);
> + return -1;
> + }
> + } else if (g_strcmp0(tokens[0], "trace-all") == 0) {
> + if (!qemu_plugin_bool_parse(tokens[0], tokens[1],
> + &trace_all_insns)) {
> + fprintf(stderr, "boolean argument parsing failed: %s\n",
> opt);
> + return -1;
> + }
> + } else {
> + fprintf(stderr, "option parsing failed: %s\n", opt);
> + return -1;
> + }
> + }
> +
> + states = qemu_plugin_scoreboard_new(sizeof(struct cpu_state));
> + last_pc = qemu_plugin_scoreboard_u64_in_struct(states, struct cpu_state,
> + last_pc);
> + from_pc = qemu_plugin_scoreboard_u64_in_struct(states, struct cpu_state,
> + from_pc);
> + has_from = qemu_plugin_scoreboard_u64_in_struct(states, struct cpu_state,
> + has_from);
> +
> + qemu_plugin_register_vcpu_discon_cb(id, QEMU_PLUGIN_DISCON_ALL,
> + vcpu_discon);
> + qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
> +
> + return 0;
> +}
> diff --git a/tests/tcg/plugins/meson.build b/tests/tcg/plugins/meson.build
> index 61a007d9e7..561584159e 100644
> --- a/tests/tcg/plugins/meson.build
> +++ b/tests/tcg/plugins/meson.build
> @@ -1,6 +1,6 @@
> t = []
> if get_option('plugins')
> - foreach i : ['bb', 'empty', 'inline', 'insn', 'mem', 'reset', 'syscall',
> 'patch']
> + foreach i : ['bb', 'discons', 'empty', 'inline', 'insn', 'mem', 'reset',
> 'syscall', 'patch']
> if host_os == 'windows'
> t += shared_module(i, files(i + '.c') +
> '../../../contrib/plugins/win32_linker.c',
> include_directories: '../../../include/qemu',
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
