Richard Henderson <richard.hender...@linaro.org> writes:
> As noted by qemu-plugins.h, enum qemu_plugin_cb_flags is
> currently unused -- plugins can neither read nor write
> guest registers.
No objection to this - although we hopefully will introduce the ability
to read registers at some point. I saw no indication that the ability to
mark helpers for that is going away, just the mechanism is changing?
>
> Cc: Alex Bennée <alex.ben...@linaro.org>
> Tested-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> ---
> accel/tcg/plugin-helpers.h | 1 -
> include/qemu/plugin.h | 1 -
> accel/tcg/plugin-gen.c | 8 ++++----
> plugins/core.c | 30 ++++++------------------------
> 4 files changed, 10 insertions(+), 30 deletions(-)
>
> diff --git a/accel/tcg/plugin-helpers.h b/accel/tcg/plugin-helpers.h
> index 1916ee7920..853bd21677 100644
> --- a/accel/tcg/plugin-helpers.h
> +++ b/accel/tcg/plugin-helpers.h
> @@ -1,5 +1,4 @@
> #ifdef CONFIG_PLUGIN
> -/* Note: no TCG flags because those are overwritten later */
> DEF_HELPER_2(plugin_vcpu_udata_cb, void, i32, ptr)
> DEF_HELPER_4(plugin_vcpu_mem_cb, void, i32, i32, i64, ptr)
> #endif
> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
> index c5a79a89f0..0fefbc6084 100644
> --- a/include/qemu/plugin.h
> +++ b/include/qemu/plugin.h
> @@ -79,7 +79,6 @@ enum plugin_dyn_cb_subtype {
> struct qemu_plugin_dyn_cb {
> union qemu_plugin_cb_sig f;
> void *userp;
> - unsigned tcg_flags;
> enum plugin_dyn_cb_subtype type;
> /* @rw applies to mem callbacks only (both regular and inline) */
> enum qemu_plugin_mem_rw rw;
> diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
> index 48bd2f36f0..88e25c6df9 100644
> --- a/accel/tcg/plugin-gen.c
> +++ b/accel/tcg/plugin-gen.c
> @@ -384,7 +384,7 @@ static TCGOp *copy_st_ptr(TCGOp **begin_op, TCGOp *op)
> }
>
> static TCGOp *copy_call(TCGOp **begin_op, TCGOp *op, void *empty_func,
> - void *func, unsigned tcg_flags, int *cb_idx)
> + void *func, int *cb_idx)
> {
> /* copy all ops until the call */
> do {
> @@ -411,7 +411,7 @@ static TCGOp *copy_call(TCGOp **begin_op, TCGOp *op, void
> *empty_func,
> tcg_debug_assert(i < MAX_OPC_PARAM_ARGS);
> }
> op->args[*cb_idx] = (uintptr_t)func;
> - op->args[*cb_idx + 1] = tcg_flags;
> + op->args[*cb_idx + 1] = (*begin_op)->args[*cb_idx + 1];
This confuses me. We are dropping tcg_flags because we aren't using them
but why are we copying the next args from begin_op? Should we have been
doing that before?
>
> return op;
> }
> @@ -438,7 +438,7 @@ static TCGOp *append_udata_cb(const struct
> qemu_plugin_dyn_cb *cb,
>
> /* call */
> op = copy_call(&begin_op, op, HELPER(plugin_vcpu_udata_cb),
> - cb->f.vcpu_udata, cb->tcg_flags, cb_idx);
> + cb->f.vcpu_udata, cb_idx);
>
> return op;
> }
> @@ -489,7 +489,7 @@ static TCGOp *append_mem_cb(const struct
> qemu_plugin_dyn_cb *cb,
> if (type == PLUGIN_GEN_CB_MEM) {
> /* call */
> op = copy_call(&begin_op, op, HELPER(plugin_vcpu_mem_cb),
> - cb->f.vcpu_udata, cb->tcg_flags, cb_idx);
> + cb->f.vcpu_udata, cb_idx);
> }
>
> return op;
> diff --git a/plugins/core.c b/plugins/core.c
> index 55d188af51..e1bcdb570d 100644
> --- a/plugins/core.c
> +++ b/plugins/core.c
> @@ -295,33 +295,15 @@ void plugin_register_inline_op(GArray **arr,
> dyn_cb->inline_insn.imm = imm;
> }
>
> -static inline uint32_t cb_to_tcg_flags(enum qemu_plugin_cb_flags flags)
> -{
> - uint32_t ret;
> -
> - switch (flags) {
> - case QEMU_PLUGIN_CB_RW_REGS:
> - ret = 0;
> - break;
> - case QEMU_PLUGIN_CB_R_REGS:
> - ret = TCG_CALL_NO_WG;
> - break;
> - case QEMU_PLUGIN_CB_NO_REGS:
> - default:
> - ret = TCG_CALL_NO_RWG;
> - }
> - return ret;
> -}
> -
> -inline void
> -plugin_register_dyn_cb__udata(GArray **arr,
> - qemu_plugin_vcpu_udata_cb_t cb,
> - enum qemu_plugin_cb_flags flags, void *udata)
> +void plugin_register_dyn_cb__udata(GArray **arr,
> + qemu_plugin_vcpu_udata_cb_t cb,
> + enum qemu_plugin_cb_flags flags,
> + void *udata)
> {
> struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr);
>
> dyn_cb->userp = udata;
> - dyn_cb->tcg_flags = cb_to_tcg_flags(flags);
> + /* Note flags are discarded as unused. */
"currently unused" would be a slightly more hopeful statement ;-)
> dyn_cb->f.vcpu_udata = cb;
> dyn_cb->type = PLUGIN_CB_REGULAR;
> }
> @@ -336,7 +318,7 @@ void plugin_register_vcpu_mem_cb(GArray **arr,
>
> dyn_cb = plugin_get_dyn_cb(arr);
> dyn_cb->userp = udata;
> - dyn_cb->tcg_flags = cb_to_tcg_flags(flags);
> + /* Note flags are discarded as unused. */
> dyn_cb->type = PLUGIN_CB_REGULAR;
> dyn_cb->rw = rw;
> dyn_cb->f.generic = cb;
--
Alex Bennée
