On 9/2/24 10:52, Alex Bennée wrote:
Pierrick Bouvier <pierrick.bouv...@linaro.org> writes:

Hi Xingran,

On 9/2/24 03:42, Alex Bennée wrote:
Xingran Wang <wangxingran123...@outlook.com> writes:

Currently, the instruction count obtained by plugins using the translation
block execution callback is larger than the actual value. Adding callbacks
in cpu_restore_state_from_tb() and cpu_io_recompile() allows plugins to
correct the instruction count when exiting a translation block
mid-execution, properly subtracting the excess unexecuted
instructions.
This smells like exposing two much of the TCG internals to the
plugin
mechanism. You can already detect when we don't reach the end of a block
of instructions by instrumentation as I did in:


I agree that this is definitely a QEMU implementation "detail", and
should not be a concern for end users.

The documentation already warns that all instructions may not execute,
and that in this case, it's better to instrument them directly,
instead of TB:
https://www.qemu.org/docs/master/devel/tcg-plugins.html#translation-blocks.

Finally, even if we integrated an API like what you propose in this
patch, I think it would be very easy for plugins writers to make a
mistake using it, as you need to keep track of everything yourself.

If we want to stay on the topic of this patch, one direction that
would be better in my opinion is a "after_tb_exec" API, where the TB
passed in parameter is guaranteed to have all its instructions that
were executed (not interrupted).

Or indeed resolves with the current PC at the "end" of the TB where it
gets to. QEMU could keep track of that easily enough as the recompile
and bus fault paths are slow paths anyway. It would be tricky to support
inline for that though.

As TB is exposed internally I think we'd just need to set a flag and
call out. Maybe an API like:

   /**
    * typedef qemu_plugin_vcpu_tb_end_cb_t - vcpu callback at end of block
    * @vcpu_index: the current vcpu context
    * @pc: the next PC
    * @insns: instructions executed in block
    * @userdata: a pointer to some user data supplied when the callback
    * was registered.
    */
   typedef void (*qemu_plugin_vcpu_tb_end_cb_t)(unsigned int vcpu_index,
                                                uint64_t next_pc,
                                                size_t n_insns,
                                                void *userdata);

   /**
    * qemu_plugin_register_vcpu_tb_exec_end_cb() - register execution callback 
at end of TB
    * @tb: the opaque qemu_plugin_tb handle for the translation
    * @cb: callback function
    * @flags: does the plugin read or write the CPU's registers?
    * @userdata: any plugin data to pass to the @cb?
    *
    * The @cb function is called every time a translated unit executes.
    */
   QEMU_PLUGIN_API
   void qemu_plugin_register_vcpu_tb_exec_end_cb(struct qemu_plugin_tb *tb,
                                                  qemu_plugin_vcpu_tb_end_cb_t 
cb,
                                                  enum qemu_plugin_cb_flags 
flags,
                                                  void *userdata);

I think the tricky bit would be getting TCG to emit the callback code
for the last instruction before the
tcg_gen_exit_tb/tcg_gen_lookup_and_goto_ptr bits but after whatever else
it has done to execute the instruction.

I don't think we could easily support inline ops at tb end though.

Richard,

What do you think?
I think this will miss all exceptions raised in the middle of the block.
I don't think it will be reliable at all.


r~

Reply via email to