Hi, Pierrick,
October 21, 2024 at 11:59 PM, "Pierrick Bouvier" wrote:
> On 10/21/24 14:02, Julian Ganz wrote:
> > 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;
> };
I have considered such an enum, as an input for the callback, as a
parameter of the registration function, and both. Of course, if you were
to add a selection parameter for the registration function, you likely
want OR-able flags.
An additional input for the callback type would obviously require a new
function type just for that callback. Since the callbacks are somewhat
similar to the VCPU init, exit, resume, ... ones it felt appropriate
to use the same function type for all of them.
As for the registration, it may make the registration a bit more
convenient and maybe keep the API clutter a bit lower, but not by that
much.
> /* 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,
> ... */
I don't think this is a good idea.
Traps are just too diverse, imo there is too little overlap between
different architectures, with the sole exception maybe being the PC
prior to the trap. "Interrupt id" sounds like a reasonably common
concept, but then you would need to define a mapping for each and every
architecture. What integer type do you use? In RISC-V, for example,
exceptions and interrupt "ids" are differentiated via the most
significant bit. Dou keep that or do you zero it? And then there's
ring/privilage mode, cause (sometimes for each mode), ...
It would also complicate call sites for hooks in target code. You'd
either need awkwardly long function signitures or setup code for that
struct. Both are things you don't want, as a hook call site should
never distract from the actual logic surrounding them. You could
probably have something reasonable in Rust, using a builder/command
pattern. But in C this would require too much boiler plate code than
I'd be comfortable with.
Regards,
Julian
