plugins: fix optimization in plugin_gen_disable_mem_helpers
We were mistakenly checking tcg_ctx->plugin_insn as a canary to know whether the TB had emitted helpers that might have accessed memory. The problem is that tcg_ctx->plugin_insn gets updated on every instruction in the TB, which results in us wrongly performing the optimization (i.e. not clearing cpu->plugin_mem_cbs) way too often, since it's not rare that the last instruction in the TB doesn't use helpers. Fix it by tracking a per-TB canary. While at it, expand documentation. Related: #1381 Signed-off-by: Emilio Cota <cota@braap.org> Message-Id: <20230108164731.61469-2-cota@braap.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Message-Id: <20230124180127.1881110-32-alex.bennee@linaro.org>
This commit is contained in:
parent
2bbbc1be8d
commit
3fd62e73ad
@ -579,7 +579,8 @@ static void inject_mem_helper(TCGOp *begin_op, GArray *arr)
|
||||
* is possible that the code we generate after the instruction is
|
||||
* dead, we also add checks before generating tb_exit etc.
|
||||
*/
|
||||
static void inject_mem_enable_helper(struct qemu_plugin_insn *plugin_insn,
|
||||
static void inject_mem_enable_helper(struct qemu_plugin_tb *ptb,
|
||||
struct qemu_plugin_insn *plugin_insn,
|
||||
TCGOp *begin_op)
|
||||
{
|
||||
GArray *cbs[2];
|
||||
@ -599,6 +600,7 @@ static void inject_mem_enable_helper(struct qemu_plugin_insn *plugin_insn,
|
||||
rm_ops(begin_op);
|
||||
return;
|
||||
}
|
||||
ptb->mem_helper = true;
|
||||
|
||||
arr = g_array_sized_new(false, false,
|
||||
sizeof(struct qemu_plugin_dyn_cb), n_cbs);
|
||||
@ -626,15 +628,22 @@ void plugin_gen_disable_mem_helpers(void)
|
||||
{
|
||||
TCGv_ptr ptr;
|
||||
|
||||
if (likely(tcg_ctx->plugin_insn == NULL ||
|
||||
!tcg_ctx->plugin_insn->mem_helper)) {
|
||||
/*
|
||||
* We could emit the clearing unconditionally and be done. However, this can
|
||||
* be wasteful if for instance plugins don't track memory accesses, or if
|
||||
* most TBs don't use helpers. Instead, emit the clearing iff the TB calls
|
||||
* helpers that might access guest memory.
|
||||
*
|
||||
* Note: we do not reset plugin_tb->mem_helper here; a TB might have several
|
||||
* exit points, and we want to emit the clearing from all of them.
|
||||
*/
|
||||
if (!tcg_ctx->plugin_tb->mem_helper) {
|
||||
return;
|
||||
}
|
||||
ptr = tcg_const_ptr(NULL);
|
||||
tcg_gen_st_ptr(ptr, cpu_env, offsetof(CPUState, plugin_mem_cbs) -
|
||||
offsetof(ArchCPU, env));
|
||||
tcg_temp_free_ptr(ptr);
|
||||
tcg_ctx->plugin_insn->mem_helper = false;
|
||||
}
|
||||
|
||||
static void plugin_gen_tb_udata(const struct qemu_plugin_tb *ptb,
|
||||
@ -682,14 +691,14 @@ static void plugin_gen_mem_inline(const struct qemu_plugin_tb *ptb,
|
||||
inject_inline_cb(cbs, begin_op, op_rw);
|
||||
}
|
||||
|
||||
static void plugin_gen_enable_mem_helper(const struct qemu_plugin_tb *ptb,
|
||||
static void plugin_gen_enable_mem_helper(struct qemu_plugin_tb *ptb,
|
||||
TCGOp *begin_op, int insn_idx)
|
||||
{
|
||||
struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
|
||||
inject_mem_enable_helper(insn, begin_op);
|
||||
inject_mem_enable_helper(ptb, insn, begin_op);
|
||||
}
|
||||
|
||||
static void plugin_gen_disable_mem_helper(const struct qemu_plugin_tb *ptb,
|
||||
static void plugin_gen_disable_mem_helper(struct qemu_plugin_tb *ptb,
|
||||
TCGOp *begin_op, int insn_idx)
|
||||
{
|
||||
struct qemu_plugin_insn *insn = g_ptr_array_index(ptb->insns, insn_idx);
|
||||
@ -750,7 +759,7 @@ static void pr_ops(void)
|
||||
#endif
|
||||
}
|
||||
|
||||
static void plugin_gen_inject(const struct qemu_plugin_tb *plugin_tb)
|
||||
static void plugin_gen_inject(struct qemu_plugin_tb *plugin_tb)
|
||||
{
|
||||
TCGOp *op;
|
||||
int insn_idx = -1;
|
||||
@ -870,6 +879,7 @@ bool plugin_gen_tb_start(CPUState *cpu, const DisasContextBase *db,
|
||||
ptb->haddr1 = db->host_addr[0];
|
||||
ptb->haddr2 = NULL;
|
||||
ptb->mem_only = mem_only;
|
||||
ptb->mem_helper = false;
|
||||
|
||||
plugin_gen_empty_callback(PLUGIN_GEN_FROM_TB);
|
||||
}
|
||||
|
@ -118,7 +118,10 @@ struct qemu_plugin_insn {
|
||||
void *haddr;
|
||||
GArray *cbs[PLUGIN_N_CB_TYPES][PLUGIN_N_CB_SUBTYPES];
|
||||
bool calls_helpers;
|
||||
|
||||
/* if set, the instruction calls helpers that might access guest memory */
|
||||
bool mem_helper;
|
||||
|
||||
bool mem_only;
|
||||
};
|
||||
|
||||
@ -158,6 +161,10 @@ struct qemu_plugin_tb {
|
||||
void *haddr1;
|
||||
void *haddr2;
|
||||
bool mem_only;
|
||||
|
||||
/* if set, the TB calls helpers that might access guest memory */
|
||||
bool mem_helper;
|
||||
|
||||
GArray *cbs[PLUGIN_N_CB_SUBTYPES];
|
||||
};
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user