[PATCH v2] plugins: fix qemu_plugin_reset

[PATCH v2] plugins: fix qemu_plugin_reset

[Pierrick Bouvier]

34e5e1 refactored the plugin context initialization. After this change,
tcg_ctx->plugin_insn is not reset inconditionnally anymore, but only if
one plugin at least is active.

When uninstalling the last plugin active, we stopped reinitializing
tcg_ctx->plugin_insn, which leads to memory callbacks being emitted.
This results in an error as they don't appear in a plugin op sequence as
expected.

The correct fix is to make sure we reset plugin translation variables
after current block translation ends. This way, we can catch any
potential misuse of those after a given block, in more than fixing the
current bug.

v2: do not reset tcg_ctx->plugin_tb as it gets reused between
translations.

Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2570
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 accel/tcg/plugin-gen.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 2ee4c22befd..0f47bfbb489 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -467,4 +467,8 @@ void plugin_gen_tb_end(CPUState *cpu, size_t num_insns)
 
     /* inject the instrumentation at the appropriate places */
     plugin_gen_inject(ptb);
+
+    /* reset plugin translation state (plugin_tb is reused between blocks) */
+    tcg_ctx->plugin_db = NULL;
+    tcg_ctx->plugin_insn = NULL;
 }
-- 
2.39.5

Re: [PATCH v2] plugins: fix qemu_plugin_reset

[Robbin Ehn]

On Tue, Oct 15, 2024 at 2:38 AM Pierrick Bouvier
<pierrick.bouvier@linaro.org> wrote:
>
> 34e5e1 refactored the plugin context initialization. After this change,
> tcg_ctx->plugin_insn is not reset inconditionnally anymore, but only if
> one plugin at least is active.
>
> When uninstalling the last plugin active, we stopped reinitializing
> tcg_ctx->plugin_insn, which leads to memory callbacks being emitted.
> This results in an error as they don't appear in a plugin op sequence as
> expected.
>
> The correct fix is to make sure we reset plugin translation variables
> after current block translation ends. This way, we can catch any
> potential misuse of those after a given block, in more than fixing the
> current bug.
>
> v2: do not reset tcg_ctx->plugin_tb as it gets reused between
> translations.
>
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2570
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>  accel/tcg/plugin-gen.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
> index 2ee4c22befd..0f47bfbb489 100644
> --- a/accel/tcg/plugin-gen.c
> +++ b/accel/tcg/plugin-gen.c
> @@ -467,4 +467,8 @@ void plugin_gen_tb_end(CPUState *cpu, size_t num_insns)
>
>      /* inject the instrumentation at the appropriate places */
>      plugin_gen_inject(ptb);
> +
> +    /* reset plugin translation state (plugin_tb is reused between blocks) */
> +    tcg_ctx->plugin_db = NULL;
> +    tcg_ctx->plugin_insn = NULL;
>  }
> --
> 2.39.5
>
>

Thanks!

Tested-by: Robbin Ehn <rehn@rivosinc.com>

Re: [PATCH v2] plugins: fix qemu_plugin_reset

[Alex Bennée]

Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:

> 34e5e1 refactored the plugin context initialization. After this change,
> tcg_ctx->plugin_insn is not reset inconditionnally anymore, but only if
> one plugin at least is active.
>
> When uninstalling the last plugin active, we stopped reinitializing
> tcg_ctx->plugin_insn, which leads to memory callbacks being emitted.
> This results in an error as they don't appear in a plugin op sequence as
> expected.
>
> The correct fix is to make sure we reset plugin translation variables
> after current block translation ends. This way, we can catch any
> potential misuse of those after a given block, in more than fixing the
> current bug.
>
> v2: do not reset tcg_ctx->plugin_tb as it gets reused between
> translations.

For reference put version information bellow

---

and then the git tools will trim it out of the commit message.

Queued to plugins/next, thanks.

>
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2570
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>  accel/tcg/plugin-gen.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
> index 2ee4c22befd..0f47bfbb489 100644
> --- a/accel/tcg/plugin-gen.c
> +++ b/accel/tcg/plugin-gen.c
> @@ -467,4 +467,8 @@ void plugin_gen_tb_end(CPUState *cpu, size_t num_insns)
>  
>      /* inject the instrumentation at the appropriate places */
>      plugin_gen_inject(ptb);
> +
> +    /* reset plugin translation state (plugin_tb is reused between blocks) */
> +    tcg_ctx->plugin_db = NULL;
> +    tcg_ctx->plugin_insn = NULL;
>  }

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v2] plugins: fix qemu_plugin_reset

[Pierrick Bouvier]

On 10/21/24 03:22, Alex Bennée wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> 
>> 34e5e1 refactored the plugin context initialization. After this change,
>> tcg_ctx->plugin_insn is not reset inconditionnally anymore, but only if
>> one plugin at least is active.
>>
>> When uninstalling the last plugin active, we stopped reinitializing
>> tcg_ctx->plugin_insn, which leads to memory callbacks being emitted.
>> This results in an error as they don't appear in a plugin op sequence as
>> expected.
>>
>> The correct fix is to make sure we reset plugin translation variables
>> after current block translation ends. This way, we can catch any
>> potential misuse of those after a given block, in more than fixing the
>> current bug.
>>
>> v2: do not reset tcg_ctx->plugin_tb as it gets reused between
>> translations.
> 
> For reference put version information bellow
> 
> ---
> 
> and then the git tools will trim it out of the commit message.
> 
> Queued to plugins/next, thanks.
> 

Thanks, I was not sure how to deal with versioning for individual 
patches 👍.

>>
>> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2570
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>   accel/tcg/plugin-gen.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
>> index 2ee4c22befd..0f47bfbb489 100644
>> --- a/accel/tcg/plugin-gen.c
>> +++ b/accel/tcg/plugin-gen.c
>> @@ -467,4 +467,8 @@ void plugin_gen_tb_end(CPUState *cpu, size_t num_insns)
>>   
>>       /* inject the instrumentation at the appropriate places */
>>       plugin_gen_inject(ptb);
>> +
>> +    /* reset plugin translation state (plugin_tb is reused between blocks) */
>> +    tcg_ctx->plugin_db = NULL;
>> +    tcg_ctx->plugin_insn = NULL;
>>   }
>