Re: [PATCH 25/26] tcg: exclude lookup_tb_ptr from helper instrumentation

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 1/10/23 09:39, Alex Bennée wrote:
>> From: Emilio Cota <cota@braap.org>
>> It is internal to TCG and therefore we know it does not
>> access guest memory.
>> Related: #1381
>> Signed-off-by: Emilio Cota <cota@braap.org>
>> Message-Id: <20230108164731.61469-4-cota@braap.org>
>> Signed-off-by: Alex Bennée <alex.bennee@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.

>
>
> r~


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro