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

[Gerd Hoffmann <kraxel@redhat.com>]

On Thu, Sep 02, 2021 at 03:09:15PM +0200, Richard Henderson wrote:
> On 8/31/21 2:15 PM, Gerd Hoffmann wrote:
> > diff --git a/target/i386/helper.c b/target/i386/helper.c
> > index 533b29cb91b6..100add713c5d 100644
> > --- a/target/i386/helper.c
> > +++ b/target/i386/helper.c
> > @@ -103,7 +103,7 @@ void x86_cpu_set_a20(X86CPU *cpu, int a20_state)
> >           /* when a20 is changed, all the MMU mappings are invalid, so
> >              we must flush everything */
> > -        tlb_flush(cs);
> > +        tcg.tlb_flush(cs);
> 
> I think this is a mistake.
> 
> (1) If tcg module is not enabled, we should be able to make a direct call.
> 
> So IMO we want to retain the direct function call syntax in all the uses.  I
> think you want to put wrapper functions doing the indirection somewhere --
> possibly tcg-module.c.

Hmm, when we want avoid indirection I guess it makes sense to use inline
wrappers in tcg-module.h

How about the patch below? (proof-of-concept for tlb_flush, on top of
this series)?

thanks,
  Gerd

From 22f5a216f410fccb769d0f7496c3c36f4b131833 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Tue, 28 Sep 2021 11:51:12 +0200
Subject: [PATCH] tlb_flush inline wrapper

---
 include/exec/exec-all.h  |  2 ++
 include/tcg/tcg-module.h | 13 +++++++++++++
 meson.build              |  3 ++-
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 608d768a4371..72e4e3b5bb89 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -160,7 +160,9 @@ void tlb_flush_page_all_cpus_synced(CPUState *src, target_ulong addr);
  * so this is generally safe. If more selective flushing is required
  * use one of the other functions for efficiency.
  */
+#ifdef TCG_DIRECT_CALL
 void tlb_flush(CPUState *cpu);
+#endif
 /**
  * tlb_flush_all_cpus:
  * @cpu: src CPU of the flush
diff --git a/include/tcg/tcg-module.h b/include/tcg/tcg-module.h
index e9c0615b51d9..b3c0f53ea9f3 100644
--- a/include/tcg/tcg-module.h
+++ b/include/tcg/tcg-module.h
@@ -1,6 +1,10 @@
 #ifndef TCG_MODULE_H
 #define TCG_MODULE_H
 
+#if defined(CONFIG_TCG_BUILTIN) || defined(TCG_MODULE)
+# define TCG_DIRECT_CALL 1
+#endif
+
 #include "exec/exec-all.h"
 
 struct TCGModuleOps {
@@ -21,4 +25,13 @@ struct TCGModuleOps {
 };
 extern struct TCGModuleOps tcg;
 
+#ifndef TCG_DIRECT_CALL
+
+static inline void tlb_flush(CPUState *cpu)
+{
+    tcg.tlb_flush(cpu);
+}
+
+#endif
+
 #endif /* TCG_MODULE_H */
diff --git a/meson.build b/meson.build
index 15ef4d3c4187..afe07e7d59c3 100644
--- a/meson.build
+++ b/meson.build
@@ -2317,8 +2317,9 @@ subdir('tests/qtest/libqos')
 subdir('tests/qtest/fuzz')
 
 # accel modules
+tcg_module_cflags = declare_dependency(compile_args: '-DTCG_MODULE=1')
 tcg_real_module_ss = ss.source_set()
-tcg_real_module_ss.add_all(when: 'CONFIG_TCG_MODULAR', if_true: tcg_module_ss)
+tcg_real_module_ss.add_all(when: ['CONFIG_TCG_MODULAR', tcg_module_cflags], if_true: tcg_module_ss)
 specific_ss.add_all(when: 'CONFIG_TCG_BUILTIN', if_true: tcg_module_ss)
 target_modules += { 'accel' : { 'qtest': qtest_module_ss,
                                 'tcg': tcg_real_module_ss }}
-- 
2.31.1