From 047e2bd3383ba488f928b335f9d99cef3a216418 Mon Sep 17 00:00:00 2001 From: Emilio Cota Date: Tue, 24 Jan 2023 18:01:20 +0000 Subject: thread: de-const qemu_spin_destroy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewed-by: Alex Bennée Signed-off-by: Emilio Cota Reviewed-by: Richard Henderson Message-Id: <20230111151628.320011-4-cota@braap.org> Signed-off-by: Alex Bennée Message-Id: <20230124180127.1881110-29-alex.bennee@linaro.org> --- include/qemu/thread.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'include') diff --git a/include/qemu/thread.h b/include/qemu/thread.h index 7c6703bce3..7841084199 100644 --- a/include/qemu/thread.h +++ b/include/qemu/thread.h @@ -237,11 +237,10 @@ static inline void qemu_spin_init(QemuSpin *spin) #endif } -/* const parameter because the only purpose here is the TSAN annotation */ -static inline void qemu_spin_destroy(const QemuSpin *spin) +static inline void qemu_spin_destroy(QemuSpin *spin) { #ifdef CONFIG_TSAN - __tsan_mutex_destroy((void *)spin, __tsan_mutex_not_static); + __tsan_mutex_destroy(spin, __tsan_mutex_not_static); #endif } -- cgit 1.4.1 From 3fd62e73ad8193b58a1bc15ad6f6d8b4f284f6a7 Mon Sep 17 00:00:00 2001 From: Emilio Cota Date: Tue, 24 Jan 2023 18:01:23 +0000 Subject: plugins: fix optimization in plugin_gen_disable_mem_helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Message-Id: <20230108164731.61469-2-cota@braap.org> Signed-off-by: Alex Bennée Message-Id: <20230124180127.1881110-32-alex.bennee@linaro.org> --- accel/tcg/plugin-gen.c | 26 ++++++++++++++++++-------- include/qemu/plugin.h | 7 +++++++ 2 files changed, 25 insertions(+), 8 deletions(-) (limited to 'include') diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c index c7d6514840..17a686bd9e 100644 --- a/accel/tcg/plugin-gen.c +++ b/accel/tcg/plugin-gen.c @@ -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); } diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h index a772e14193..e0ebedef84 100644 --- a/include/qemu/plugin.h +++ b/include/qemu/plugin.h @@ -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]; }; -- cgit 1.4.1 From 17083f6fa6e02326f1c1e8b67b8b282480d013d2 Mon Sep 17 00:00:00 2001 From: Emilio Cota Date: Tue, 24 Jan 2023 18:01:25 +0000 Subject: tcg: exclude non-memory effecting helpers from instrumentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are actually a whole bunch of helpers that don't affect memory that we shouldn't instrument. They are helpfully identified by the TCG_CALL_NO_SIDE_EFFECTS flag which marks out lookup_tb_ptr as well as a lot of the maths helpers. To avoid the string compare we introduce a new flag for plugin internals so we skip that too. Related: #1381 Signed-off-by: Emilio Cota Message-Id: <20230108164731.61469-4-cota@braap.org> [AJB: updated to skip all no SE plugins, add flag for plugin helper] Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Message-Id: <20230124180127.1881110-34-alex.bennee@linaro.org> --- accel/tcg/plugin-helpers.h | 4 ++-- include/tcg/tcg.h | 2 ++ tcg/tcg.c | 6 ++++-- 3 files changed, 8 insertions(+), 4 deletions(-) (limited to 'include') diff --git a/accel/tcg/plugin-helpers.h b/accel/tcg/plugin-helpers.h index 9829abe4a9..8e685e0654 100644 --- a/accel/tcg/plugin-helpers.h +++ b/accel/tcg/plugin-helpers.h @@ -1,4 +1,4 @@ #ifdef CONFIG_PLUGIN -DEF_HELPER_FLAGS_2(plugin_vcpu_udata_cb, TCG_CALL_NO_RWG, void, i32, ptr) -DEF_HELPER_FLAGS_4(plugin_vcpu_mem_cb, TCG_CALL_NO_RWG, void, i32, i32, i64, ptr) +DEF_HELPER_FLAGS_2(plugin_vcpu_udata_cb, TCG_CALL_NO_RWG | TCG_CALL_PLUGIN, void, i32, ptr) +DEF_HELPER_FLAGS_4(plugin_vcpu_mem_cb, TCG_CALL_NO_RWG | TCG_CALL_PLUGIN, void, i32, i32, i64, ptr) #endif diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h index 6f497172f8..8dc291d030 100644 --- a/include/tcg/tcg.h +++ b/include/tcg/tcg.h @@ -405,6 +405,8 @@ typedef TCGv_ptr TCGv_env; #define TCG_CALL_NO_SIDE_EFFECTS 0x0004 /* Helper is G_NORETURN. */ #define TCG_CALL_NO_RETURN 0x0008 +/* Helper is part of Plugins. */ +#define TCG_CALL_PLUGIN 0x0010 /* convenience version of most used call flags */ #define TCG_CALL_NO_RWG TCG_CALL_NO_READ_GLOBALS diff --git a/tcg/tcg.c b/tcg/tcg.c index d502327be2..fd557d55d3 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -1674,8 +1674,10 @@ void tcg_gen_callN(void *func, TCGTemp *ret, int nargs, TCGTemp **args) op = tcg_op_alloc(INDEX_op_call, total_args); #ifdef CONFIG_PLUGIN - /* detect non-plugin helpers */ - if (tcg_ctx->plugin_insn && unlikely(strncmp(info->name, "plugin_", 7))) { + /* Flag helpers that may affect guest state */ + if (tcg_ctx->plugin_insn && + !(info->flags & TCG_CALL_PLUGIN) && + !(info->flags & TCG_CALL_NO_SIDE_EFFECTS)) { tcg_ctx->plugin_insn->calls_helpers = true; } #endif -- cgit 1.4.1 From 882f5b1b4418cfa8f65ff2be52286247db40d152 Mon Sep 17 00:00:00 2001 From: Emilio Cota Date: Tue, 24 Jan 2023 18:01:26 +0000 Subject: cpu-exec: assert that plugin_mem_cbs is NULL after execution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: #1381 Signed-off-by: Emilio Cota Message-Id: <20230108165107.62488-1-cota@braap.org> [AJB: manually applied follow-up fix] Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20230124180127.1881110-35-alex.bennee@linaro.org> --- accel/tcg/cpu-exec.c | 2 ++ include/qemu/plugin.h | 4 ++++ 2 files changed, 6 insertions(+) (limited to 'include') diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index 25ec73ef9a..9c857eeb07 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -504,6 +504,7 @@ static void cpu_exec_exit(CPUState *cpu) if (cc->tcg_ops->cpu_exec_exit) { cc->tcg_ops->cpu_exec_exit(cpu); } + QEMU_PLUGIN_ASSERT(cpu->plugin_mem_cbs == NULL); } void cpu_exec_step_atomic(CPUState *cpu) @@ -980,6 +981,7 @@ cpu_exec_loop(CPUState *cpu, SyncClocks *sc) cpu_loop_exec_tb(cpu, tb, pc, &last_tb, &tb_exit); + QEMU_PLUGIN_ASSERT(cpu->plugin_mem_cbs == NULL); /* Try to align the host and virtual clocks if the guest is in advance */ align_clocks(sc, cpu); diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h index e0ebedef84..fb338ba576 100644 --- a/include/qemu/plugin.h +++ b/include/qemu/plugin.h @@ -59,6 +59,8 @@ get_plugin_meminfo_rw(qemu_plugin_meminfo_t i) #ifdef CONFIG_PLUGIN extern QemuOptsList qemu_plugin_opts; +#define QEMU_PLUGIN_ASSERT(cond) g_assert(cond) + static inline void qemu_plugin_add_opts(void) { qemu_add_opts(&qemu_plugin_opts); @@ -250,6 +252,8 @@ void qemu_plugin_user_postfork(bool is_child); #else /* !CONFIG_PLUGIN */ +#define QEMU_PLUGIN_ASSERT(cond) + static inline void qemu_plugin_add_opts(void) { } -- cgit 1.4.1