Re: [RFC PATCH v2 0/7] tcg-plugins: add hooks for interrupts, exceptions and traps

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

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