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

["Alex Bennée" <alex.bennee@linaro.org>]

"Julian Ganz" <nenut@skiff.uberspace.de> writes:

> Hi, Pierrick,
>
> resent as I was too stupid to hit reply instead of reply-all.
>
> October 22, 2024 at 11:15 PM, "Pierrick Bouvier" wrote:
>> 
>> On 10/22/24 01:21, Julian Ganz wrote:
>> 
>> > 
>> > Hi, Pierrick,
>> >  October 21, 2024 at 11:59 PM, "Pierrick Bouvier" wrote:
>> >
<snip>
>> >  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), ...
>> > 
>> I didn't want to open the per architecture pandora box :).
>> I don't think the plugin API itself should deal with per architecture
>> details like meaning of a given id. I was just thinking to push this
>> "raw" information to the plugin, that may/may not use architecture
>> specific knowledge to do its work. We already have plugins that have
>> similar per architecture knowledge (contrib/plugins/howvec.c) and
>> it's ok in some specific cases.
>
> But how would such an interface look? The last PC aside, what would you
> include, and how? A GArray with named items that are itself just opaque
> blobs?
>
> And what would be the benefit compared to just querying the respective
> target specific registers through qemu_plugin_read_register? Which btw.
> is what we were going to do for our use-case. Even the example you
> brought up (howvec) uses querying functions and doesn't expect to get
> all the info via parameters.

I think the register access probably provides everything you need. Some
targets provide a wider access than other though. I haven't looked at
the Risc V code but certainly the Arm code exposes pretty much all
system registers to the gdbstub (and hence the plugin interface).

If there is example of state that isn't accessible this way then I'd
like to know it.

>> But having something like from/to address seems useful to start. Even if we don't provide it for all events yet, it's ok.
>
> Yes, I certainly see the advantages of having either the last PC or the
> would-be-next PC as they are sufficiently universal. You can usually
> retrieve them from target-specific registers, but that may be more
> complicated in practice. In the case of RISC-V for example, the value
> of the EPC differs between interrupts and exceptions.
>
> That PC value should also be easy enough to obtain at the hook call
> sites. We only need to store the (old) PC before doing the setup. The
> "to-address" is the current PC at the time the callback is invoked.
> Anything else would be a bug. I was going to write that you can
> already query that in a plugin through a dedicated helper function
> but apparently I misremembered.
>
> I'll include this in the next iteration.

There are some dragons with pc/npc as each front-end deals with it its
own way and some targets have delay slots which makes things even
messier.

>
>> > 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.
>> > 
>> We can have one "builder" function per data type, with fixed parameters (no varargs), it's reasonable and would scale well with new entries/data information.
>
> I'm still not on board on preparing a more complex data type. For the
> next iteration I'd rather stick to a simple function receiving the
> "type" of event and the PCs. That may not be extensible, but I don't see
> any benefit in shoehorning inheritelntly target-specifc information into
> a complex struct.
>
> If this is a hard requirement, I'll of course still do so.

No lets keep it simple for the first iteration. We can also expand the
API later and bump the API versions as appropriate.

>
> Regards,
> Julian

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro