Re: [PATCH 06/22] plugins: Create TCGHelperInfo for all out-of-line callbacks

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

On 3/16/24 05:57, Richard Henderson wrote:
> TCGHelperInfo includes the ABI for every function call.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/qemu/plugin.h |  1 +
>   plugins/core.c        | 51 ++++++++++++++++++++++++++++++++++++++-----
>   2 files changed, 46 insertions(+), 6 deletions(-)
> 
> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
> index 143262dca8..793c44f1f2 100644
> --- a/include/qemu/plugin.h
> +++ b/include/qemu/plugin.h
> @@ -92,6 +92,7 @@ struct qemu_plugin_dyn_cb {
>       union {
>           struct {
>               union qemu_plugin_cb_sig f;
> +            TCGHelperInfo *info;
>           } regular;
>           struct {
>               qemu_plugin_u64 entry;
> diff --git a/plugins/core.c b/plugins/core.c
> index 837c373690..b0a2e80874 100644
> --- a/plugins/core.c
> +++ b/plugins/core.c
> @@ -338,12 +338,26 @@ void plugin_register_dyn_cb__udata(GArray **arr,
>                                      enum qemu_plugin_cb_flags flags,
>                                      void *udata)
>   {
> -    struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr);
> +    static TCGHelperInfo info[3] = {
> +        [QEMU_PLUGIN_CB_NO_REGS].flags = TCG_CALL_NO_RWG | TCG_CALL_PLUGIN,
> +        [QEMU_PLUGIN_CB_R_REGS].flags = TCG_CALL_NO_WG | TCG_CALL_PLUGIN,
> +        [QEMU_PLUGIN_CB_RW_REGS].flags = TCG_CALL_PLUGIN,
> +        /*
> +         * Match qemu_plugin_vcpu_udata_cb_t:
> +         *   void (*)(uint32_t, void *)
> +

Any chance we could have a static assert ensuring this?
I know it's possible in C11, but I don't know if glib offers something 
for this.

          */
> +        [0 ... 2].typemask = (dh_typemask(void, 0) |
> +                              dh_typemask(i32, 1) |
> +                              dh_typemask(ptr, 2))
> +    };
>   
> +    struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr);
>       dyn_cb->userp = udata;
> -    /* Note flags are discarded as unused. */
> -    dyn_cb->regular.f.vcpu_udata = cb;
>       dyn_cb->type = PLUGIN_CB_REGULAR;
> +    dyn_cb->regular.f.vcpu_udata = cb;
> +
> +    assert((unsigned)flags < ARRAY_SIZE(info));
> +    dyn_cb->regular.info = &info[flags];
>   }
>   
>   void plugin_register_vcpu_mem_cb(GArray **arr,
> @@ -352,14 +366,39 @@ void plugin_register_vcpu_mem_cb(GArray **arr,
>                                    enum qemu_plugin_mem_rw rw,
>                                    void *udata)
>   {
> -    struct qemu_plugin_dyn_cb *dyn_cb;
> +    /*
> +     * Expect that the underlying type for enum qemu_plugin_meminfo_t
> +     * is either int32_t or uint32_t, aka int or unsigned int.
> +     */
> +    QEMU_BUILD_BUG_ON(
> +        !__builtin_types_compatible_p(qemu_plugin_meminfo_t, uint32_t) &&
> +        !__builtin_types_compatible_p(qemu_plugin_meminfo_t, int32_t));
>   
> -    dyn_cb = plugin_get_dyn_cb(arr);
> +    static TCGHelperInfo info[3] = {
> +        [QEMU_PLUGIN_CB_NO_REGS].flags = TCG_CALL_NO_RWG | TCG_CALL_PLUGIN,
> +        [QEMU_PLUGIN_CB_R_REGS].flags = TCG_CALL_NO_WG | TCG_CALL_PLUGIN,
> +        [QEMU_PLUGIN_CB_RW_REGS].flags = TCG_CALL_PLUGIN,
> +        /*
> +         * Match qemu_plugin_vcpu_mem_cb_t:
> +         *   void (*)(uint32_t, qemu_plugin_meminfo_t, uint64_t, void *)
> +         */
> +        [0 ... 2].typemask =
> +            (dh_typemask(void, 0) |
> +             dh_typemask(i32, 1) |
> +             (__builtin_types_compatible_p(qemu_plugin_meminfo_t, uint32_t)
> +              ? dh_typemask(i32, 2) : dh_typemask(s32, 2)) |
> +             dh_typemask(i64, 3) |
> +             dh_typemask(ptr, 4))
> +    };
> +
> +    struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr);
>       dyn_cb->userp = udata;
> -    /* Note flags are discarded as unused. */
>       dyn_cb->type = PLUGIN_CB_REGULAR;
>       dyn_cb->rw = rw;
>       dyn_cb->regular.f.vcpu_mem = cb;
> +
> +    assert((unsigned)flags < ARRAY_SIZE(info));
> +    dyn_cb->regular.info = &info[flags];
>   }
>   
>   /*

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