From: "Alex Bennée" <alex.bennee@linaro.org>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: qemu-devel@nongnu.org, "Philippe Mathieu-Daudé" <f4bug@amsat.org>
Subject: Re: [PATCH v7 04/27] plugins: Drop tcg_flags from struct qemu_plugin_dyn_cb
Date: Wed, 02 Jun 2021 10:22:22 +0100 [thread overview]
Message-ID: <87h7igbou1.fsf@linaro.org> (raw)
In-Reply-To: <20210601150106.12761-5-richard.henderson@linaro.org>
Richard Henderson <richard.henderson@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.bennee@linaro.org>
> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Richard Henderson <richard.henderson@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
next prev parent reply other threads:[~2021-06-02 9:26 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-01 15:00 [PATCH v7 00/27] TCI fixes and cleanups Richard Henderson
2021-06-01 15:00 ` [PATCH v7 01/27] tcg: Combine dh_is_64bit and dh_is_signed to dh_typecode Richard Henderson
2021-06-01 15:00 ` [PATCH v7 02/27] tcg: Add tcg_call_flags Richard Henderson
2021-06-01 15:00 ` [PATCH v7 03/27] accel/tcg/plugin-gen: Drop inline markers Richard Henderson
2021-06-01 15:00 ` [PATCH v7 04/27] plugins: Drop tcg_flags from struct qemu_plugin_dyn_cb Richard Henderson
2021-06-02 9:22 ` Alex Bennée [this message]
2021-06-02 16:02 ` Richard Henderson
2021-06-01 15:00 ` [PATCH v7 05/27] accel/tcg: Add tcg call flags to plugins helpers Richard Henderson
2021-06-01 15:00 ` [PATCH v7 06/27] tcg: Store the TCGHelperInfo in the TCGOp for call Richard Henderson
2021-06-01 15:00 ` [PATCH v7 07/27] tcg: Add tcg_call_func Richard Henderson
2021-06-01 15:00 ` [PATCH v7 08/27] tcg: Build ffi data structures for helpers Richard Henderson
2021-06-01 15:00 ` [PATCH v7 09/27] tcg/tci: Improve tcg_target_call_clobber_regs Richard Henderson
2021-06-02 17:59 ` Philippe Mathieu-Daudé
2021-06-01 15:00 ` [PATCH v7 10/27] tcg/tci: Move call-return regs to end of tcg_target_reg_alloc_order Richard Henderson
2021-06-01 15:00 ` [PATCH v7 11/27] tcg/tci: Use ffi for calls Richard Henderson
2021-06-02 10:31 ` Alex Bennée
2021-06-01 15:00 ` [PATCH v7 12/27] tcg/tci: Reserve r13 for a temporary Richard Henderson
2021-06-19 15:50 ` Philippe Mathieu-Daudé
2021-06-01 15:00 ` [PATCH v7 13/27] tcg/tci: Emit setcond before brcond Richard Henderson
2021-06-19 15:50 ` Philippe Mathieu-Daudé
2021-06-01 15:00 ` [PATCH v7 14/27] tcg/tci: Remove tci_write_reg Richard Henderson
2021-06-01 15:00 ` [PATCH v7 15/27] tcg/tci: Change encoding to uint32_t units Richard Henderson
2021-06-12 10:21 ` Philippe Mathieu-Daudé
2021-06-12 15:40 ` Richard Henderson
2021-06-19 17:48 ` Philippe Mathieu-Daudé
2021-06-19 18:05 ` Richard Henderson
2021-06-01 15:00 ` [PATCH v7 16/27] tcg/tci: Implement goto_ptr Richard Henderson
2021-06-12 9:45 ` Philippe Mathieu-Daudé
2021-06-01 15:00 ` [PATCH v7 17/27] tcg/tci: Implement movcond Richard Henderson
2021-06-01 15:00 ` [PATCH v7 18/27] tcg/tci: Implement andc, orc, eqv, nand, nor Richard Henderson
2021-06-01 15:00 ` [PATCH v7 19/27] tcg/tci: Implement extract, sextract Richard Henderson
2021-06-01 15:00 ` [PATCH v7 20/27] tcg/tci: Implement clz, ctz, ctpop Richard Henderson
2021-06-01 15:01 ` [PATCH v7 21/27] tcg/tci: Implement mulu2, muls2 Richard Henderson
2021-06-01 15:01 ` [PATCH v7 22/27] tcg/tci: Implement add2, sub2 Richard Henderson
2021-06-12 9:36 ` Philippe Mathieu-Daudé
2021-06-01 15:01 ` [PATCH v7 23/27] tcg/tci: Split out tci_qemu_ld, tci_qemu_st Richard Henderson
2021-06-02 18:01 ` Philippe Mathieu-Daudé
2021-06-01 15:01 ` [PATCH v7 24/27] Revert "tcg/tci: Use exec/cpu_ldst.h interfaces" Richard Henderson
2021-06-12 9:24 ` Philippe Mathieu-Daudé
2021-06-01 15:01 ` [PATCH v7 25/27] tcg/tci: Remove the qemu_ld/st_type macros Richard Henderson
2021-06-02 18:03 ` Philippe Mathieu-Daudé
2021-06-01 15:01 ` [PATCH v7 26/27] tcg/tci: Use {set,clear}_helper_retaddr Richard Henderson
2021-06-12 9:24 ` Philippe Mathieu-Daudé
2021-06-01 15:01 ` [PATCH v7 27/27] tests/tcg: Increase timeout for TCI Richard Henderson
2021-06-02 9:26 ` Alex Bennée
2021-06-01 15:30 ` [PATCH v7 00/27] TCI fixes and cleanups no-reply
2021-06-19 17:42 ` Philippe Mathieu-Daudé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87h7igbou1.fsf@linaro.org \
--to=alex.bennee@linaro.org \
--cc=f4bug@amsat.org \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.