On 10/21/24 14:02, Julian Ganz wrote:
Hi, Pierrick,

October 21, 2024 at 8:00 PM, "Pierrick Bouvier" wrote:
I agree it would be useful. Beyond the scope of this series, it would be
nice if we could add a control flow related API instead of asking to
plugins to do it themselves.

If we would provide something like this, is there still a value to add
an API to detect interrupt/exceptions/traps events?

Note: It's not a critic against what you sent, just an open question on
*why* it's useful to access this QEMU implementation related information
vs something more generic.

The motivation for this API is a plugin that simulates a RISC-V tracing
unit (and produces a trace). For that we actually also needed to
track the "regular" control flow, i.e. find out whether a branch was
taken or where a jump went. That wasn't hard, especially considering
that the TCG API already gives you (more or less) basic blocks. Still,
we ended up tracing every instruction because that made some of the logic
much simpler and easier to reason about.

We realized that we need a trap API because they:
* can occur at any time/point of execusion
* usually come with additional effects such as mode changes.


Thanks for sharing your insights.
I think there is definitely value in what you offer, and I'm trying to think how we could extend it in the future easily, without having another callback when a new event appear. In my experience on plugins, the least callbacks we have, and the simpler they are, the better it is.

Maybe we could have a single API like:

enum qemu_plugin_cf_event_type {
        QEMU_PLUGIN_CF_INTERRUPT;
        QEMU_PLUGIN_CF_TRAP;
        QEMU_PLUGIN_CF_SEMIHOSTING;
};

/* Sum type, a.k.a. "Rust-like" enum */
typedef struct {
    enum qemu_plugin_cf_event_type ev;
    union {
        data_for_interrupt interrupt;
        data_for_trap trap;
        data_for_semihosting semihosting;
} qemu_plugin_cf_event;
/* data_for_... could contain things like from/to addresses, interrupt id, ... */

...

void on_cf_event(qemu_plugin_cf_event ev)
{
        switch (ev.type) {
                case QEMU_PLUGIN_CF_TRAP:
                        ...
                case QEMU_PLUGIN_CF_SEMIHOSTING:
                        ...
                default:
                        g_assert_not_reached();
        }
}

/* a plugin can register to one or several event - we could provide a QEMU_PLUGIN_CF_ALL for plugins tracking all events. */
qemu_plugin_register_cf_cb(QEMU_PLUGIN_CF_TRAP, &on_cf_event);
qemu_plugin_register_cf_cb(QEMU_PLUGIN_CF_SEMIHOSTING, &on_cf_event);

This way, a single callback can be registered for one or several events. And in the future, we are free to attach more data for every event, and add other events (TB_FALLTHROUGH, TB_JUMP, etc).

Helpers for discerning whether an instruction is a jump, a branch
instruction or something else would certainly be helpful if you wanted
cross-platform control flow tracing of some sort, but afaik given such
helpers you would just need to check the last instruction in a
translation block and check where the PC goes after that. Additional
callbacks for specifically this situation strike me as a bit
excessive.

But I could be wrong about that.


You're right, and the current cflow plugin is more a demonstration of using existing API than an efficient solution to this problem. For cflow detection specifically, I think we can do better, by adding instrumentation right where we chain/jump between tb, and of course, tracking other events like you did in this series.

Regards,
Julian

Reply via email to