[PATCH 05/11] accel/tcg: Move post-load tb_flush to vm_change_state hook

Richard Henderson posted 11 patches 5 months, 1 week 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>
There is a newer version of this series
[PATCH 05/11] accel/tcg: Move post-load tb_flush to vm_change_state hook
Posted by Richard Henderson 5 months, 1 week ago
We need not call tb_flush once per cpu, only once per vmload.
Move the call from cpu_common_post_load to a tcg-specific
vm_change_state_handler.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/tcg-all.c  | 21 +++++++++++++++++++++
 hw/core/cpu-system.c |  8 --------
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index 5125e1a4e2..a0bc0e58c7 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -38,6 +38,8 @@
 #include "qemu/target-info.h"
 #ifndef CONFIG_USER_ONLY
 #include "hw/boards.h"
+#include "exec/tb-flush.h"
+#include "system/runstate.h"
 #endif
 #include "accel/accel-ops.h"
 #include "accel/accel-cpu-ops.h"
@@ -82,6 +84,23 @@ static void tcg_accel_instance_init(Object *obj)
 
 bool one_insn_per_tb;
 
+#ifndef CONFIG_USER_ONLY
+static void tcg_vm_change_state(void *opaque, bool running, RunState state)
+{
+    if (state == RUN_STATE_RESTORE_VM) {
+        /*
+         * loadvm will update the content of RAM, bypassing the usual
+         * mechanisms that ensure we flush TBs for writes to memory
+         * we've translated code from, so we must flush all TBs.
+         *
+         * vm_stop() has just stopped all cpus, so we are exclusive.
+         */
+        assert(!running);
+        tb_flush__exclusive();
+    }
+}
+#endif
+
 static int tcg_init_machine(AccelState *as, MachineState *ms)
 {
     TCGState *s = TCG_STATE(as);
@@ -124,6 +143,8 @@ static int tcg_init_machine(AccelState *as, MachineState *ms)
     default:
         g_assert_not_reached();
     }
+
+    qemu_add_vm_change_state_handler(tcg_vm_change_state, NULL);
 #endif
 
     tcg_allowed = true;
diff --git a/hw/core/cpu-system.c b/hw/core/cpu-system.c
index a975405d3a..c9099c6e7a 100644
--- a/hw/core/cpu-system.c
+++ b/hw/core/cpu-system.c
@@ -207,14 +207,6 @@ static int cpu_common_post_load(void *opaque, int version_id)
         cpu->interrupt_request &= ~0x01;
 
         tlb_flush(cpu);
-
-        /*
-         * loadvm has just updated the content of RAM, bypassing the
-         * usual mechanisms that ensure we flush TBs for writes to
-         * memory we've translated code from. So we must flush all TBs,
-         * which will now be stale.
-         */
-        tb_flush(cpu);
     }
 
     return 0;
-- 
2.43.0
Re: [PATCH 05/11] accel/tcg: Move post-load tb_flush to vm_change_state hook
Posted by Peter Xu 5 months ago
On Sat, Sep 06, 2025 at 07:18:14AM +0200, Richard Henderson wrote:
> We need not call tb_flush once per cpu, only once per vmload.
> Move the call from cpu_common_post_load to a tcg-specific
> vm_change_state_handler.

The change looks correct.  Though the commit message here implies a
conversion from per-cpu flush to per-system flush, which IMHO is only part
of the change.

IIUC, the change is better than that, because previously when the flush is
in post_load(), it means the per-cpu flush will also happen in a live
migration, but here AFAIU the flush is only needed through a HMP
loadvm-styled migration, where there must be some stale data in the first
place. In case of live migrations there isn't any prior data to flush,
iiuc.

Maybe it would be nice to mention the other side of the changes in the
commit message too?  No strong feelings, though.

Thanks,

-- 
Peter Xu
Re: [PATCH 05/11] accel/tcg: Move post-load tb_flush to vm_change_state hook
Posted by Pierrick Bouvier 5 months, 1 week ago
On 2025-09-06 07:18, Richard Henderson wrote:
> We need not call tb_flush once per cpu, only once per vmload.
> Move the call from cpu_common_post_load to a tcg-specific
> vm_change_state_handler.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/tcg-all.c  | 21 +++++++++++++++++++++
>   hw/core/cpu-system.c |  8 --------
>   2 files changed, 21 insertions(+), 8 deletions(-)

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>