Re: [PATCH 4/7] plugins: Introduce TCGCPUOps callbacks for mid-tb register reads

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

On 4/15/24 21:06, Richard Henderson wrote:
> Certain target registers are not updated continuously within
> the translation block.  For normal exception handling we use
> unwind info to re-generate the correct value when required.
> Leverage that same info for reading those registers for plugins.
> 
> All targets will need updating for these new callbacks.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/hw/core/tcg-cpu-ops.h | 13 +++++++++++++
>   plugins/api.c                 | 36 +++++++++++++++++++++++++++++++++--
>   2 files changed, 47 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
> index bf8ff8e3ee..e954d83edf 100644
> --- a/include/hw/core/tcg-cpu-ops.h
> +++ b/include/hw/core/tcg-cpu-ops.h
> @@ -49,6 +49,19 @@ struct TCGCPUOps {
>       /** @debug_excp_handler: Callback for handling debug exceptions */
>       void (*debug_excp_handler)(CPUState *cpu);
>   
> +    /**
> +     * @plugin_need_unwind_for_reg:
> +     * True if unwind info needed for reading reg.
> +     */
> +    bool (*plugin_need_unwind_for_reg)(CPUState *cpu, int reg);
> +    /**
> +     * @plugin_unwind_read_reg:
> +     * Like CPUClass.gdb_read_register, but for registers that require
> +     * regeneration using unwind info, like in @restore_state_to_opc.
> +     */
> +    int (*plugin_unwind_read_reg)(CPUState *cpu, GByteArray *buf, int reg,
> +                                  const TranslationBlock *tb,
> +                                  const uint64_t *data);
>   #ifdef NEED_CPU_H
>   #ifdef CONFIG_USER_ONLY
>       /**
> diff --git a/plugins/api.c b/plugins/api.c
> index 3912c9cc8f..3543647a89 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -40,10 +40,12 @@
>   #include "qemu/plugin.h"
>   #include "qemu/log.h"
>   #include "tcg/tcg.h"
> +#include "tcg/insn-start-words.h"
>   #include "exec/exec-all.h"
>   #include "exec/gdbstub.h"
>   #include "exec/ram_addr.h"
>   #include "disas/disas.h"
> +#include "hw/core/tcg-cpu-ops.h"
>   #include "plugin.h"
>   #ifndef CONFIG_USER_ONLY
>   #include "qemu/plugin-memory.h"
> @@ -454,9 +456,39 @@ GArray *qemu_plugin_get_registers(void)
>   
>   int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf)
>   {
> -    g_assert(current_cpu);
> +    CPUState *cs;
> +    uintptr_t ra;
> +    int regno;
>   
> -    return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg));
> +    assert(current_cpu);
> +    cs = current_cpu;
> +    ra = cs->neg.plugin_ra;
> +    regno = GPOINTER_TO_INT(reg);
> +
> +    /*
> +     * When plugin_ra is 0, we have no unwind info.  This will be true for
> +     * TB callbacks that happen before any insns of the TB have started.
> +     */
> +    if (ra) {
> +        const TCGCPUOps *tcg_ops = cs->cc->tcg_ops;
> +
> +        /*
> +         * For plugins in the middle of the TB, we may need to locate
> +         * and use unwind data to reconstruct a register value.
> +         * Usually this required for the PC, but there may be others.
> +         */
> +        if (tcg_ops->plugin_need_unwind_for_reg &&
> +            tcg_ops->plugin_need_unwind_for_reg(cs, regno)) {
> +            uint64_t data[TARGET_INSN_START_WORDS];
> +            const TranslationBlock *tb;
> +
> +            tb = cpu_unwind_state_data(cs, ra, data);
> +            assert(tb);
> +            return tcg_ops->plugin_unwind_read_reg(cs, buf, regno, tb, data);
> +        }
> +    }
> +
> +    return gdb_read_register(cs, buf, regno);
>   }
>   
>   struct qemu_plugin_scoreboard *qemu_plugin_scoreboard_new(size_t element_size)

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>