Hi Pierrick,
May 13, 2025 at 2:25 AM, Pierrick Bouvier wrote:
> On 5/11/25 6:14 AM, Julian Ganz wrote:
> > +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.
I was likely thinking about other types of discontinuities we may add in
the future. But yes, it's probably not worth thinking about that now.
> > [...]
> > + 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.
Mh... Thanks for the hint. I'll have a closer look later.
I also wonder whether this could also be useful for solving the issue we
run into with virtual memory: TBs being reused in a context different
from the one may have a different addresses. That's why we introduced
the compare-addr-bits argument.
Regards,
Julian
