Alex Bennée <alex.ben...@linaro.org> writes:
> Richard Henderson <richard.hender...@linaro.org> writes:
>
>> On 1/10/23 09:39, Alex Bennée wrote:
>>> From: Emilio Cota <c...@braap.org>
>>> It is internal to TCG and therefore we know it does not
>>> access guest memory.
>>> Related: #1381
>>> Signed-off-by: Emilio Cota <c...@braap.org>
>>> Message-Id: <20230108164731.61469-4-c...@braap.org>
>>> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
>>> ---
>>> tcg/tcg.c | 6 ++++--
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>> diff --git a/tcg/tcg.c b/tcg/tcg.c
>>> index da91779890..ee67eefc0c 100644
>>> --- a/tcg/tcg.c
>>> +++ b/tcg/tcg.c
>>> @@ -1652,8 +1652,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 are not internal to TCG */
>>> + if (tcg_ctx->plugin_insn &&
>>> + strncmp(info->name, "plugin_", 7) &&
>>> + strcmp(info->name, "lookup_tb_ptr")) {
>>> tcg_ctx->plugin_insn->calls_helpers = true;
>>> }
>>> #endif
>>
>> I think this should be detected with
>>
>> !(info->flags & TCG_CALL_NO_SIDE_EFFECTS)
>>
>> i.e., side-effects, which in this case is the possibility of a fault.
>
> That implies that:
>
> 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)
>
> should be the _SE variants as well right? They do have side-effects but
> not in guest state and they shouldn't cause a fault.
Hmm but we don't want them to go away. Maybe something like:
3 files changed, 7 insertions(+), 5 deletions(-)
accel/tcg/plugin-helpers.h | 4 ++--
include/tcg/tcg.h | 2 ++
tcg/tcg.c | 6 +++---
modified 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
modified 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
modified tcg/tcg.c
@@ -1652,10 +1652,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
- /* flag helpers that are not internal to TCG */
+ /* Flag helpers that may affect guest state */
if (tcg_ctx->plugin_insn &&
- strncmp(info->name, "plugin_", 7) &&
- strcmp(info->name, "lookup_tb_ptr")) {
+ !(info->flags & TCG_CALL_PLUGIN) &&
+ !(info->flags & TCG_CALL_NO_SIDE_EFFECTS)) {
tcg_ctx->plugin_insn->calls_helpers = true;
}
#endif
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
