[v2] accel/tcg: Improve tb_flush usage

[PATCH v2 02/12] accel/tcg: Split out tb_flush__exclusive

Posted by Richard Henderson 5 days, 13 hours ago

Expose a routine to be called when no cpus are running.
Simplify the do_tb_flush run_on_cpu callback, because
that is explicitly called with start_exclusive; there
is no need for the mmap_lock as well.

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/tb-flush.h | 13 +++++++++++++
 accel/tcg/tb-maint.c    | 33 ++++++++++++++++++++-------------
 2 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/include/exec/tb-flush.h b/include/exec/tb-flush.h
index 142c240d94..5076b13bbd 100644
--- a/include/exec/tb-flush.h
+++ b/include/exec/tb-flush.h
@@ -8,6 +8,19 @@
 #ifndef _TB_FLUSH_H_
 #define _TB_FLUSH_H_
 
+/**
+ * tb_flush__exclusive() - flush all translation blocks
+ *
+ * Used to flush all the translation blocks in the system.
+ * Sometimes it is simpler to flush everything than work out which
+ * individual translations are now invalid and ensure they are
+ * not called anymore.
+ *
+ * Must be called from an exclusive context, e.g. start_exclusive
+ * or vm_stop.
+ */
+void tb_flush__exclusive(void);
+
 /**
  * tb_flush() - flush all translation blocks
  * @cs: CPUState (must be valid, but treated as anonymous pointer)
diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
index 0048316f99..71216fa75e 100644
--- a/accel/tcg/tb-maint.c
+++ b/accel/tcg/tb-maint.c
@@ -36,6 +36,9 @@
 #include "internal-common.h"
 #ifdef CONFIG_USER_ONLY
 #include "user/page-protection.h"
+#define runstate_is_running()  true
+#else
+#include "system/runstate.h"
 #endif
 
 
@@ -756,17 +759,18 @@ static void tb_remove(TranslationBlock *tb)
 }
 #endif /* CONFIG_USER_ONLY */
 
-/* flush all the translation blocks */
-static void do_tb_flush(CPUState *cpu, run_on_cpu_data tb_flush_count)
+/*
+ * Flush all the translation blocks.
+ * Must be called from a context in which no cpus are running,
+ * e.g. start_exclusive() or vm_stop().
+ */
+void tb_flush__exclusive(void)
 {
-    bool did_flush = false;
+    CPUState *cpu;
 
-    mmap_lock();
-    /* If it is already been done on request of another CPU, just retry. */
-    if (tb_ctx.tb_flush_count != tb_flush_count.host_int) {
-        goto done;
-    }
-    did_flush = true;
+    assert(tcg_enabled());
+    assert(!runstate_is_running() ||
+           (current_cpu && cpu_in_serial_context(current_cpu)));
 
     CPU_FOREACH(cpu) {
         tcg_flush_jmp_cache(cpu);
@@ -778,11 +782,14 @@ static void do_tb_flush(CPUState *cpu, run_on_cpu_data tb_flush_count)
     tcg_region_reset_all();
     /* XXX: flush processor icache at this point if cache flush is expensive */
     qatomic_inc(&tb_ctx.tb_flush_count);
+    qemu_plugin_flush_cb();
+}
 
-done:
-    mmap_unlock();
-    if (did_flush) {
-        qemu_plugin_flush_cb();
+static void do_tb_flush(CPUState *cpu, run_on_cpu_data tb_flush_count)
+{
+    /* If it is already been done on request of another CPU, just retry. */
+    if (tb_ctx.tb_flush_count == tb_flush_count.host_int) {
+        tb_flush__exclusive();
     }
 }
 
-- 
2.43.0

Re: [PATCH v2 02/12] accel/tcg: Split out tb_flush__exclusive

Posted by Philippe Mathieu-Daudé 5 days, 6 hours ago

On 23/9/25 04:39, Richard Henderson wrote:
> Expose a routine to be called when no cpus are running.
> Simplify the do_tb_flush run_on_cpu callback, because
> that is explicitly called with start_exclusive; there
> is no need for the mmap_lock as well.
> 
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/exec/tb-flush.h | 13 +++++++++++++
>   accel/tcg/tb-maint.c    | 33 ++++++++++++++++++++-------------
>   2 files changed, 33 insertions(+), 13 deletions(-)
> 
> diff --git a/include/exec/tb-flush.h b/include/exec/tb-flush.h
> index 142c240d94..5076b13bbd 100644
> --- a/include/exec/tb-flush.h
> +++ b/include/exec/tb-flush.h
> @@ -8,6 +8,19 @@
>   #ifndef _TB_FLUSH_H_
>   #define _TB_FLUSH_H_
>   
> +/**
> + * tb_flush__exclusive() - flush all translation blocks
> + *
> + * Used to flush all the translation blocks in the system.
> + * Sometimes it is simpler to flush everything than work out which
> + * individual translations are now invalid and ensure they are
> + * not called anymore.
> + *
> + * Must be called from an exclusive context, e.g. start_exclusive
> + * or vm_stop.

Worth mentioning it shouldn't be called from target code?

> + */
> +void tb_flush__exclusive(void);

Re: [PATCH v2 02/12] accel/tcg: Split out tb_flush__exclusive

Posted by Philippe Mathieu-Daudé 5 days, 8 hours ago

On 23/9/25 04:39, Richard Henderson wrote:
> Expose a routine to be called when no cpus are running.
> Simplify the do_tb_flush run_on_cpu callback, because
> that is explicitly called with start_exclusive; there
> is no need for the mmap_lock as well.
> 
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/exec/tb-flush.h | 13 +++++++++++++
>   accel/tcg/tb-maint.c    | 33 ++++++++++++++++++++-------------
>   2 files changed, 33 insertions(+), 13 deletions(-)


> diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
> index 0048316f99..71216fa75e 100644
> --- a/accel/tcg/tb-maint.c
> +++ b/accel/tcg/tb-maint.c
> @@ -36,6 +36,9 @@
>   #include "internal-common.h"
>   #ifdef CONFIG_USER_ONLY
>   #include "user/page-protection.h"
> +#define runstate_is_running()  true

Hmm, acceptable.

> +#else
> +#include "system/runstate.h"
>   #endif
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>