Re: [PATCH 10/29] tcg_funcs: Add tlb_flush to TCGModuleOps

[Gerd Hoffmann]

> > +#ifdef TCG_DIRECT_CALL
> >   void tlb_flush(CPUState *cpu);
> > +#endif
> 
> I'm pretty sure you can drop these ifdefs.  Just because there's a regular
> declaration for a function doesn't mean a subsequent inline definition does
> not apply.

Couldn't get this work.  With "static inline" I get declaration
mismatch, with only "inline" I get duplicate symbols.

> And even if that didn't work, I'd be willing to trade inline expansion for
> not adding lots of ifdefs...

That works.  Patch below (replacement for this patch).

TODO: Not sure whenever "check function pointer before call" or
"function pointers to stubs" is better.  Right now the patch has both
which clearly doesn't make sense.  Comments on that are welcome.

take care,
  Gerd

>From fb1d9521fba11cf51f2719b587deeab79b7110ac Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Tue, 27 Jul 2021 12:01:24 +0200
Subject: [PATCH] tcg_funcs: Add tlb_flush to TCGModuleOps

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/exec/exec-all.h     | 25 ++++++++++++-------------
 include/tcg/tcg-module.h    |  1 +
 accel/tcg/cpu-exec-common.c |  8 ++++++++
 accel/tcg/cputlb.c          |  7 +++++++
 accel/tcg/tcg-module.c      | 21 +++++++++++++++++++++
 accel/tcg/translate-all.c   |  8 --------
 6 files changed, 49 insertions(+), 21 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 9d5987ba047d..f846ab0c929d 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -25,6 +25,7 @@
 #include "exec/cpu_ldst.h"
 #endif
 #include "sysemu/cpu-timers.h"
+#include "tcg/tcg-module.h"
 
 /* allow to see translation results - the slowdown should be negligible, so we 
leave it */
 #define DEBUG_DISAS
@@ -108,6 +109,17 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
                             const char *prefix, MemoryRegion *mr);
 #endif
 
+/**
+ * tlb_flush:
+ * @cpu: CPU whose TLB should be flushed
+ *
+ * Flush the entire TLB for the specified CPU. Most CPU architectures
+ * allow the implementation to drop entries from the TLB at any time
+ * so this is generally safe. If more selective flushing is required
+ * use one of the other functions for efficiency.
+ */
+void tlb_flush(CPUState *cpu);
+
 #if !defined(CONFIG_USER_ONLY) && defined(CONFIG_TCG)
 /* cputlb.c */
 /**
@@ -150,16 +162,6 @@ void tlb_flush_page_all_cpus(CPUState *src, target_ulong 
addr);
  * the guests translation ends the TB.
  */
 void tlb_flush_page_all_cpus_synced(CPUState *src, target_ulong addr);
-/**
- * tlb_flush:
- * @cpu: CPU whose TLB should be flushed
- *
- * Flush the entire TLB for the specified CPU. Most CPU architectures
- * allow the implementation to drop entries from the TLB at any time
- * so this is generally safe. If more selective flushing is required
- * use one of the other functions for efficiency.
- */
-void tlb_flush(CPUState *cpu);
 /**
  * tlb_flush_all_cpus:
  * @cpu: src CPU of the flush
@@ -337,9 +339,6 @@ static inline void tlb_flush_page_all_cpus_synced(CPUState 
*src,
                                                   target_ulong addr)
 {
 }
-static inline void tlb_flush(CPUState *cpu)
-{
-}
 static inline void tlb_flush_all_cpus(CPUState *src_cpu)
 {
 }
diff --git a/include/tcg/tcg-module.h b/include/tcg/tcg-module.h
index 7e87aecb2357..b94bfdd362ed 100644
--- a/include/tcg/tcg-module.h
+++ b/include/tcg/tcg-module.h
@@ -2,6 +2,7 @@
 #define TCG_MODULE_H
 
 struct TCGModuleOps {
+    void (*tlb_flush)(CPUState *cpu);
 };
 extern struct TCGModuleOps tcg;
 
diff --git a/accel/tcg/cpu-exec-common.c b/accel/tcg/cpu-exec-common.c
index be6fe45aa5a8..43aae0b102c1 100644
--- a/accel/tcg/cpu-exec-common.c
+++ b/accel/tcg/cpu-exec-common.c
@@ -81,3 +81,11 @@ void cpu_loop_exit_atomic(CPUState *cpu, uintptr_t pc)
     cpu->exception_index = EXCP_ATOMIC;
     cpu_loop_exit_restore(cpu, pc);
 }
+
+/* This is a wrapper for common code that can not use CONFIG_SOFTMMU */
+void tcg_flush_softmmu_tlb(CPUState *cs)
+{
+#ifdef CONFIG_SOFTMMU
+    tlb_flush(cs);
+#endif
+}
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index b1e5471f949f..40c3d1b65ac5 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -2767,3 +2767,10 @@ uint64_t cpu_ldq_code(CPUArchState *env, abi_ptr addr)
     TCGMemOpIdx oi = make_memop_idx(MO_TEQ, cpu_mmu_index(env, true));
     return full_ldq_code(env, addr, oi, 0);
 }
+
+static void tcg_module_ops_tlb(void)
+{
+    tcg.tlb_flush = tlb_flush;
+}
+
+type_init(tcg_module_ops_tlb);
diff --git a/accel/tcg/tcg-module.c b/accel/tcg/tcg-module.c
index e864fb20c141..4d045b91647c 100644
--- a/accel/tcg/tcg-module.c
+++ b/accel/tcg/tcg-module.c
@@ -1,5 +1,26 @@
 #include "qemu/osdep.h"
 #include "tcg/tcg-module.h"
 
+static void update_cpu_stub(CPUState *cpu)
+{
+}
+
 struct TCGModuleOps tcg = {
+    .tlb_flush = update_cpu_stub,
 };
+
+/*
+ * CONFIG_TCG_MODULAR: jump through TCGModuleOps to tcg module
+ * !CONFIG_TCG or CONFIG_USER_ONLY: using the wrappers as stubs
+ * otherwise: using direct calls into tcg -> no wrappers
+ */
+#if defined(CONFIG_TCG_MODULAR) || !defined(CONFIG_TCG) || 
defined(CONFIG_USER_ONLY)
+
+void tlb_flush(CPUState *cpu)
+{
+    if (tcg.tlb_flush) {
+        tcg.tlb_flush(cpu);
+    }
+}
+
+#endif
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index fb9ebfad9e4a..9dba585ad413 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -2472,11 +2472,3 @@ int page_unprotect(target_ulong address, uintptr_t pc)
     return 0;
 }
 #endif /* CONFIG_USER_ONLY */
-
-/* This is a wrapper for common code that can not use CONFIG_SOFTMMU */
-void tcg_flush_softmmu_tlb(CPUState *cs)
-{
-#ifdef CONFIG_SOFTMMU
-    tlb_flush(cs);
-#endif
-}
-- 
2.31.1