| Message ID | 20241015003819.984601-1-pierrick.bouvier@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] plugins: fix qemu_plugin_reset | expand |
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>
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; > }
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; >> } >
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; }