On 5/11/25 6:14 AM, Julian Ganz wrote:
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. Since
instruction PCs are recorded at translation blocks translation time and
a TB may be used in multiple processes running in distinct virtual
memory, the plugin allows comparing not full addresses but a subset of
address bits via the `compare-addr-bits` option.
Signed-off-by: Julian Ganz <neither@nut.email>
---
tests/tcg/plugins/discons.c | 219 ++++++++++++++++++++++++++++++++++
tests/tcg/plugins/meson.build | 2 +-
2 files changed, 220 insertions(+), 1 deletion(-)
create mode 100644 tests/tcg/plugins/discons.c
[...]
+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);
+
+ switch (type) {
+ case QEMU_PLUGIN_DISCON_EXCEPTION:
+ /*
+ * For some types of exceptions, insn_exec will be called for the
+ * instruction that caused the exception.
+ */
+ if (addr_eq(state->last_pc, from_pc)) {
+ break;
+ }
+ __attribute__((fallthrough));
It's a bit hard to follow the codepath with the switch and the
fallthrough. Maybe we can simply use an empty if for that.
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, so we don't report this
* case.
*/
} else if (state->has_next) {
...
} else if (state->has_from) {
...
}
...
set state
...
+ default:
+ 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);
+ struct insn_data* insn = (struct insn_data *) userdata;
+
+ state->last_pc = insn->addr;
+ state->has_last = true;
+
+ if (insn->next_valid) {
+ state->from_pc = insn->next_pc;
+ }
+ state->has_from = insn->next_valid;
+
+ if (state->has_next) {
+ report_mismatch("target", vcpu_index, state->next_type, state->last_pc,
+ state->next_pc, insn->addr);
+ 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",
+ insn->addr, vcpu_index);
+ qemu_plugin_outs(report->str);
+ }
+}
+
+static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
+{
+ size_t i;
+ size_t n_insns = qemu_plugin_tb_n_insns(tb);
+ struct insn_data *udata = calloc(n_insns, sizeof(struct insn_data));
+
With this, for every TB translated, we'll perform an allocation, and
then lose track of the pointer. It's usually a pain to pass this kind of
"dynamic" information through udata.
A more elegant solution is to perform a QEMU_PLUGIN_INLINE_STORE_U64 to
store this information under a new cpu_state.current_insn field directly.
Callbacks are installed in the order you register them, so by storing
information inline *before* the insn_exec callback, it will work as
expected, as cpu_static.current_insn will be already updated.
You can find some other plugins which use this trick.
+ for (i = 0; i < n_insns; i++) {
Feel free to declare i in the loop directly.
+ struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
+ uint64_t pc = qemu_plugin_insn_vaddr(insn);
+ udata[i].addr = pc;
+ udata[i].next_pc = pc + qemu_plugin_insn_size(insn);
+ udata[i].next_valid = true;
+ qemu_plugin_register_vcpu_insn_exec_cb(insn, insn_exec,
+ QEMU_PLUGIN_CB_NO_REGS,
+ &udata[i]);
+ }
+
+ udata[n_insns - 1].next_valid = false;
+}
[...]
Otherwise, the logic of the plugin is ok for me.
Reviewed-by: Pierrick Bouvier <pierrick.bouv...@linaro.org>
