diff mbox series

[4/4] cpu-exec: assert that plugin_mem_cbs is NULL after execution

Message ID 20230108165107.62488-1-cota@braap.org (mailing list archive)
State New, archived
Headers show
Series plugin patches to fix #1381 | expand

Commit Message

Emilio Cota Jan. 8, 2023, 4:51 p.m. UTC
Fixes: #1381

Signed-off-by: Emilio Cota <cota@braap.org>
---
 accel/tcg/cpu-exec.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Alex Bennée Jan. 9, 2023, 1:52 p.m. UTC | #1
Emilio Cota <cota@braap.org> writes:

> Fixes: #1381
>
> Signed-off-by: Emilio Cota <cota@braap.org>
> ---
>  accel/tcg/cpu-exec.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 356fe348de..de4ba6e23c 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -504,6 +504,7 @@ static void cpu_exec_exit(CPUState *cpu)
>      if (cc->tcg_ops->cpu_exec_exit) {
>          cc->tcg_ops->cpu_exec_exit(cpu);
>      }
> +    g_assert(cpu->plugin_mem_cbs == NULL);
>  }
>  
>  void cpu_exec_step_atomic(CPUState *cpu)
> @@ -1031,6 +1032,7 @@ int cpu_exec(CPUState *cpu)
>  
>              cpu_loop_exec_tb(cpu, tb, pc, &last_tb, &tb_exit);
>  
> +            g_assert(cpu->plugin_mem_cbs == NULL);
>              /* Try to align the host and virtual clocks
>                 if the guest is in advance */
>              align_clocks(&sc, cpu);

Can we assert this anywhere else? This fails with non-plugin enabled
builds:

  https://gitlab.com/stsquad/qemu/-/pipelines/741478109/failures
Emilio Cota Jan. 10, 2023, 2:16 a.m. UTC | #2
On Mon, Jan 09, 2023 at 13:52:36 +0000, Alex Bennée wrote:
> Emilio Cota <cota@braap.org> writes:
> > --- a/accel/tcg/cpu-exec.c
> > +++ b/accel/tcg/cpu-exec.c
> > @@ -504,6 +504,7 @@ static void cpu_exec_exit(CPUState *cpu)
> >      if (cc->tcg_ops->cpu_exec_exit) {
> >          cc->tcg_ops->cpu_exec_exit(cpu);
> >      }
> > +    g_assert(cpu->plugin_mem_cbs == NULL);
(snip)
> > +            g_assert(cpu->plugin_mem_cbs == NULL);
> 
> Can we assert this anywhere else? This fails with non-plugin enabled
> builds:
> 
>   https://gitlab.com/stsquad/qemu/-/pipelines/741478109/failures

Ouch. Fixed in v2 with:

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index de4ba6e23c..0010b2d31e 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -504,7 +504,7 @@ static void cpu_exec_exit(CPUState *cpu)
     if (cc->tcg_ops->cpu_exec_exit) {
         cc->tcg_ops->cpu_exec_exit(cpu);
     }
-    g_assert(cpu->plugin_mem_cbs == NULL);
+    QEMU_PLUGIN_ASSERT(cpu->plugin_mem_cbs == NULL);
 }
 
 void cpu_exec_step_atomic(CPUState *cpu)
@@ -1032,7 +1032,7 @@ int cpu_exec(CPUState *cpu)
 
             cpu_loop_exec_tb(cpu, tb, pc, &last_tb, &tb_exit);
 
-            g_assert(cpu->plugin_mem_cbs == NULL);
+            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);
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index e0ebedef84..fb338ba576 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -59,6 +59,8 @@ get_plugin_meminfo_rw(qemu_plugin_meminfo_t i)
 #ifdef CONFIG_PLUGIN
 extern QemuOptsList qemu_plugin_opts;
 
+#define QEMU_PLUGIN_ASSERT(cond) g_assert(cond)
+
 static inline void qemu_plugin_add_opts(void)
 {
     qemu_add_opts(&qemu_plugin_opts);
@@ -250,6 +252,8 @@ void qemu_plugin_user_postfork(bool is_child);
 
 #else /* !CONFIG_PLUGIN */
 
+#define QEMU_PLUGIN_ASSERT(cond)
+
 static inline void qemu_plugin_add_opts(void)
 { }

Thanks,
		Emilio
Alex Bennée Jan. 10, 2023, 1:02 p.m. UTC | #3
Emilio Cota <cota@braap.org> writes:

> On Mon, Jan 09, 2023 at 13:52:36 +0000, Alex Bennée wrote:
>> Emilio Cota <cota@braap.org> writes:
>> > --- a/accel/tcg/cpu-exec.c
>> > +++ b/accel/tcg/cpu-exec.c
>> > @@ -504,6 +504,7 @@ static void cpu_exec_exit(CPUState *cpu)
>> >      if (cc->tcg_ops->cpu_exec_exit) {
>> >          cc->tcg_ops->cpu_exec_exit(cpu);
>> >      }
>> > +    g_assert(cpu->plugin_mem_cbs == NULL);
> (snip)
>> > +            g_assert(cpu->plugin_mem_cbs == NULL);
>> 
>> Can we assert this anywhere else? This fails with non-plugin enabled
>> builds:
>> 
>>   https://gitlab.com/stsquad/qemu/-/pipelines/741478109/failures
>
> Ouch. Fixed in v2 with:

Thanks, Queued to plugins/next with me manually merging the fix.
diff mbox series

Patch

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 356fe348de..de4ba6e23c 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -504,6 +504,7 @@  static void cpu_exec_exit(CPUState *cpu)
     if (cc->tcg_ops->cpu_exec_exit) {
         cc->tcg_ops->cpu_exec_exit(cpu);
     }
+    g_assert(cpu->plugin_mem_cbs == NULL);
 }
 
 void cpu_exec_step_atomic(CPUState *cpu)
@@ -1031,6 +1032,7 @@  int cpu_exec(CPUState *cpu)
 
             cpu_loop_exec_tb(cpu, tb, pc, &last_tb, &tb_exit);
 
+            g_assert(cpu->plugin_mem_cbs == NULL);
             /* Try to align the host and virtual clocks
                if the guest is in advance */
             align_clocks(&sc, cpu);