[PATCH 07/11] tcg: Clear plugin_mem_cbs on TB exit

Alex Bennée posted 11 patches 2 years, 11 months ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Thomas Huth <thuth@redhat.com>, Wainer dos Santos Moschetta <wainersm@redhat.com>, Beraldo Leal <bleal@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Peter Xu <peterx@redhat.com>, David Hildenbrand <david@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Cleber Rosa <crosa@redhat.com>
There is a newer version of this series
[PATCH 07/11] tcg: Clear plugin_mem_cbs on TB exit
Posted by Alex Bennée 2 years, 11 months ago
From: Richard Henderson <richard.henderson@linaro.org>

Do this in cpu_tb_exec (normal exit) and cpu_loop_exit (exception),
adjacent to where we reset can_do_io.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1381
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20230301024737.1210851-2-richard.henderson@linaro.org>
[AJB: use plugin_gen_disable_mem_helpers()]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 accel/tcg/cpu-exec-common.c | 3 +++
 accel/tcg/cpu-exec.c        | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/accel/tcg/cpu-exec-common.c b/accel/tcg/cpu-exec-common.c
index c7bc8c6efa..e5847e9731 100644
--- a/accel/tcg/cpu-exec-common.c
+++ b/accel/tcg/cpu-exec-common.c
@@ -21,6 +21,7 @@
 #include "sysemu/cpus.h"
 #include "sysemu/tcg.h"
 #include "exec/exec-all.h"
+#include "exec/plugin-gen.h"
 
 bool tcg_allowed;
 
@@ -65,6 +66,8 @@ void cpu_loop_exit(CPUState *cpu)
 {
     /* Undo the setting in cpu_tb_exec.  */
     cpu->can_do_io = 1;
+    /* Undo any setting in generated code. */
+    plugin_gen_disable_mem_helpers();
     siglongjmp(cpu->jmp_env, 1);
 }
 
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 56aaf58b9d..e8a48dbd93 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -39,6 +39,7 @@
 #include "exec/replay-core.h"
 #include "sysemu/tcg.h"
 #include "exec/helper-proto.h"
+#include "exec/plugin-gen.h"
 #include "tb-jmp-cache.h"
 #include "tb-hash.h"
 #include "tb-context.h"
@@ -459,6 +460,7 @@ cpu_tb_exec(CPUState *cpu, TranslationBlock *itb, int *tb_exit)
     qemu_thread_jit_execute();
     ret = tcg_qemu_tb_exec(env, tb_ptr);
     cpu->can_do_io = 1;
+    plugin_gen_disable_mem_helpers();
     /*
      * TODO: Delay swapping back to the read-write region of the TB
      * until we actually need to modify the TB.  The read-only copy,
@@ -526,7 +528,6 @@ 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)
@@ -1004,7 +1005,6 @@ 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);
-- 
2.39.2


Re: [PATCH 07/11] tcg: Clear plugin_mem_cbs on TB exit
Posted by Richard Henderson 2 years, 11 months ago
On 3/10/23 02:31, Alex Bennée wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
> 
> Do this in cpu_tb_exec (normal exit) and cpu_loop_exit (exception),
> adjacent to where we reset can_do_io.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1381
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> Message-Id: <20230301024737.1210851-2-richard.henderson@linaro.org>
> [AJB: use plugin_gen_disable_mem_helpers()]
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

This is missing Emilio's comment to remove the two existing calls to 
qemu_plugin_disable_mem_helpers which become duplicate with this change.


r~

> ---
>   accel/tcg/cpu-exec-common.c | 3 +++
>   accel/tcg/cpu-exec.c        | 4 ++--
>   2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/accel/tcg/cpu-exec-common.c b/accel/tcg/cpu-exec-common.c
> index c7bc8c6efa..e5847e9731 100644
> --- a/accel/tcg/cpu-exec-common.c
> +++ b/accel/tcg/cpu-exec-common.c
> @@ -21,6 +21,7 @@
>   #include "sysemu/cpus.h"
>   #include "sysemu/tcg.h"
>   #include "exec/exec-all.h"
> +#include "exec/plugin-gen.h"
>   
>   bool tcg_allowed;
>   
> @@ -65,6 +66,8 @@ void cpu_loop_exit(CPUState *cpu)
>   {
>       /* Undo the setting in cpu_tb_exec.  */
>       cpu->can_do_io = 1;
> +    /* Undo any setting in generated code. */
> +    plugin_gen_disable_mem_helpers();
>       siglongjmp(cpu->jmp_env, 1);
>   }
>   
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 56aaf58b9d..e8a48dbd93 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -39,6 +39,7 @@
>   #include "exec/replay-core.h"
>   #include "sysemu/tcg.h"
>   #include "exec/helper-proto.h"
> +#include "exec/plugin-gen.h"
>   #include "tb-jmp-cache.h"
>   #include "tb-hash.h"
>   #include "tb-context.h"
> @@ -459,6 +460,7 @@ cpu_tb_exec(CPUState *cpu, TranslationBlock *itb, int *tb_exit)
>       qemu_thread_jit_execute();
>       ret = tcg_qemu_tb_exec(env, tb_ptr);
>       cpu->can_do_io = 1;
> +    plugin_gen_disable_mem_helpers();
>       /*
>        * TODO: Delay swapping back to the read-write region of the TB
>        * until we actually need to modify the TB.  The read-only copy,
> @@ -526,7 +528,6 @@ 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)
> @@ -1004,7 +1005,6 @@ 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);


Re: [PATCH 07/11] tcg: Clear plugin_mem_cbs on TB exit
Posted by Richard Henderson 2 years, 11 months ago
On 3/10/23 09:39, Richard Henderson wrote:
> +    /* Undo any setting in generated code. */
> +    plugin_gen_disable_mem_helpers();

Oh!  And this is the wrong function.  Should be qemu_plugin_disable_mem_helpers.


r~

Re: [PATCH 07/11] tcg: Clear plugin_mem_cbs on TB exit
Posted by Alex Bennée 2 years, 11 months ago
Richard Henderson <richard.henderson@linaro.org> writes:

> On 3/10/23 09:39, Richard Henderson wrote:
>> +    /* Undo any setting in generated code. */
>> +    plugin_gen_disable_mem_helpers();
>
> Oh!  And this is the wrong function.  Should be
> qemu_plugin_disable_mem_helpers.

Did I miss a newer version of the patches?

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH 07/11] tcg: Clear plugin_mem_cbs on TB exit
Posted by Richard Henderson 2 years, 11 months ago
On 3/10/23 09:56, Alex Bennée wrote:
> 
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> On 3/10/23 09:39, Richard Henderson wrote:
>>> +    /* Undo any setting in generated code. */
>>> +    plugin_gen_disable_mem_helpers();
>>
>> Oh!  And this is the wrong function.  Should be
>> qemu_plugin_disable_mem_helpers.
> 
> Did I miss a newer version of the patches?

No, I never posted one, since you already said you picked it up with the requested changes.


r~