[PATCH v3 05/10] accel/tcg: Split out tb_flush__exclusive_or_serial

Richard Henderson posted 10 patches 4 days, 19 hours ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>, Nicholas Piggin <npiggin@gmail.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Laurent Vivier <laurent@vivier.eu>, Alexandre Iooss <erdnaxe@crans.org>, Mahmoud Mandour <ma.mandourr@gmail.com>, Pierrick Bouvier <pierrick.bouvier@linaro.org>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
[PATCH v3 05/10] accel/tcg: Split out tb_flush__exclusive_or_serial
Posted by Richard Henderson 4 days, 19 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 | 15 +++++++++++++++
 accel/tcg/tb-maint.c    | 39 +++++++++++++++++++++++++--------------
 2 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/include/exec/tb-flush.h b/include/exec/tb-flush.h
index 142c240d94..090ffc8818 100644
--- a/include/exec/tb-flush.h
+++ b/include/exec/tb-flush.h
@@ -8,6 +8,21 @@
 #ifndef _TB_FLUSH_H_
 #define _TB_FLUSH_H_
 
+/**
+ * tb_flush__exclusive_or_serial()
+ *
+ * Used to flush all the translation blocks in the system.  Mostly this is
+ * used to empty the code generation buffer after it is full.  Sometimes it
+ * is used when it is simpler to flush everything than work out which
+ * individual translations are now invalid.
+ *
+ * Must be called from an exclusive or serial context, e.g. start_exclusive,
+ * vm_stop, or when there is only one vcpu.  Note that start_exclusive cannot
+ * be called from within the cpu run loop, so this cannot be called from
+ * within target code.
+ */
+void tb_flush__exclusive_or_serial(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..7be9a1c4de 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
 
 
@@ -88,7 +91,10 @@ static IntervalTreeRoot tb_root;
 
 static void tb_remove_all(void)
 {
-    assert_memory_lock();
+    /*
+     * Only called from tb_flush__exclusive_or_serial, where we have already
+     * asserted that we're in an exclusive state.
+     */
     memset(&tb_root, 0, sizeof(tb_root));
 }
 
@@ -756,17 +762,19 @@ 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_or_serial(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());
+    /* Note that cpu_in_serial_context checks cpu_in_exclusive_context. */
+    assert(!runstate_is_running() ||
+           (current_cpu && cpu_in_serial_context(current_cpu)));
 
     CPU_FOREACH(cpu) {
         tcg_flush_jmp_cache(cpu);
@@ -778,11 +786,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_or_serial();
     }
 }
 
-- 
2.43.0
Re: [PATCH v3 05/10] accel/tcg: Split out tb_flush__exclusive_or_serial
Posted by Philippe Mathieu-Daudé 3 days, 14 hours ago
On 23/9/25 23:54, 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 | 15 +++++++++++++++
>   accel/tcg/tb-maint.c    | 39 +++++++++++++++++++++++++--------------
>   2 files changed, 40 insertions(+), 14 deletions(-)
> 
> diff --git a/include/exec/tb-flush.h b/include/exec/tb-flush.h
> index 142c240d94..090ffc8818 100644
> --- a/include/exec/tb-flush.h
> +++ b/include/exec/tb-flush.h
> @@ -8,6 +8,21 @@
>   #ifndef _TB_FLUSH_H_
>   #define _TB_FLUSH_H_
>   
> +/**
> + * tb_flush__exclusive_or_serial()
> + *
> + * Used to flush all the translation blocks in the system.  Mostly this is
> + * used to empty the code generation buffer after it is full.  Sometimes it
> + * is used when it is simpler to flush everything than work out which
> + * individual translations are now invalid.
> + *
> + * Must be called from an exclusive or serial context, e.g. start_exclusive,
> + * vm_stop, or when there is only one vcpu.  Note that start_exclusive cannot
> + * be called from within the cpu run loop, so this cannot be called from
> + * within target code.
> + */
> +void tb_flush__exclusive_or_serial(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..7be9a1c4de 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
>   
>   
> @@ -88,7 +91,10 @@ static IntervalTreeRoot tb_root;
>   
>   static void tb_remove_all(void)
>   {
> -    assert_memory_lock();
> +    /*
> +     * Only called from tb_flush__exclusive_or_serial, where we have already
> +     * asserted that we're in an exclusive state.
> +     */
>       memset(&tb_root, 0, sizeof(tb_root));
>   }
>   
> @@ -756,17 +762,19 @@ 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_or_serial(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;

Since reworking, here I'd add a tracing event to help debugging:

        trace_tb_flush();

> +    assert(tcg_enabled());
> +    /* Note that cpu_in_serial_context checks cpu_in_exclusive_context. */
> +    assert(!runstate_is_running() ||
> +           (current_cpu && cpu_in_serial_context(current_cpu)));
>   
>       CPU_FOREACH(cpu) {
>           tcg_flush_jmp_cache(cpu);
> @@ -778,11 +786,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_or_serial();
>       }
>   }
>