Re: [PATCH 0/4] modules: add support for target-specific modules.

["José Ricardo Ziviani" <jose.ziviani@suse.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2055 bytes --]

Hello Gerd,

On sexta-feira, 11 de junho de 2021 10:03:21 -03 Gerd Hoffmann wrote:
>   Hi,
> 
> > Are there any pending patches to handle the remaining tcg dependencies
> > in qemu?  When trying to build tcg modular (more than only
> > tcg-accel-ops*) I get lots of unresolved symbols to tcg bits which are
> > referenced directly (in cpu.c, gdbstub.c, monitor, ...).
> > 
> > The CONFIG_TCG=n case is handled either with stubs or with #ifdef
> > CONFIG_TCG, which doesn't fly for modular tcg ...
> 
> So, enough for today, to be continued next week.
> Work branch pushed to
>     https://git.kraxel.org/cgit/qemu/log/?h=sirius/modinfo-playground
> 
> Topmost patch doesn't compile but shows the build changes.

I cloned your 'sirius/modinfo-playground-good' and started playing with the 
command line options to build these modules. I would like to suggest to change 
the current "--enable-X" with "--X=[enabled,disabled,module]", that seems to 
make more sense for these modules. For instance:

$ ../configure --target-list=x86_64-softmmu --tcg=module
...
Targets and accelerators
              ...
              TCG support: YES
              TCG backend: module (x86_64)
        TCG debug enabled: NO
              target list: x86_64-softmmu
              ...

$ ../configure --target-list=x86_64-softmmu --tcg=disabled
...
Targets and accelerators
              ...
              TCG support: NO
              target list: x86_64-softmmu
              ...

$ ../configure --target-list=x86_64-softmmu --tcg=enabled # (default)
...
Targets and accelerators
              ...
              TCG support: YES
              TCG backend: native (x86_64)
        TCG debug enabled: NO
              target list: x86_64-softmmu
              ...

I attached a small patch here, just to illustrate what I'm saying. If you like 
the suggestion I can start implementing it for those modules you're currently 
working on. What do you think?

Thank you,

José R. Ziviani

> 
> take care,
>   Gerd


[-- Attachment #1.2: 0001-Add-tcg-option-to-the-build-system.patch --]
[-- Type: text/x-patch, Size: 9103 bytes --]

From 8e4cc80aae337ab8064f3ab55d3e5916186a0b19 Mon Sep 17 00:00:00 2001
From: "Jose R. Ziviani" <jziviani@suse.de>
Date: Mon, 14 Jun 2021 18:56:49 -0300
Subject: [PATCH] Add --tcg option to the build system

---
 configure            |  8 ++++++--
 meson.build          | 36 +++++++++++++++++++++++-------------
 meson_options.txt    |  3 ++-
 scripts/minikconf.py | 22 ++++++++++++++--------
 tests/meson.build    |  2 +-
 5 files changed, 46 insertions(+), 25 deletions(-)

diff --git a/configure b/configure
index 8dcb9965b2..95ed0c25c1 100755
--- a/configure
+++ b/configure
@@ -1105,7 +1105,7 @@ for opt do
   ;;
   --disable-tcg) tcg="disabled"
   ;;
-  --enable-tcg) tcg="enabled"
+  --tcg=*) tcg="$optarg"
   ;;
   --disable-malloc-trim) malloc_trim="disabled"
   ;;
@@ -1792,6 +1792,7 @@ Advanced options (experts only):
                            Default:trace-<pid>
   --disable-slirp          disable SLIRP userspace network connectivity
   --enable-tcg-interpreter enable TCI (TCG with bytecode interpreter, experimental and slow)
+  --tcg=OPTION             configure TCG accelerator [enabled|disabled|module]
   --enable-malloc-trim     enable libc malloc_trim() for memory optimization
   --oss-lib                path to OSS library
   --cpu=CPU                Build for host CPU [$cpu]
@@ -2288,7 +2289,10 @@ if test "$solaris" = "yes" ; then
   fi
 fi
 
-if test "$tcg" = "enabled"; then
+if test "$tcg" = "disabled"; then
+    debug_tcg="no"
+    tcg_interpreter="false"
+else
     git_submodules="$git_submodules tests/fp/berkeley-testfloat-3"
     git_submodules="$git_submodules tests/fp/berkeley-softfloat-3"
 fi
diff --git a/meson.build b/meson.build
index 46ebc07dbb..c372f6363e 100644
--- a/meson.build
+++ b/meson.build
@@ -195,7 +195,7 @@ elif targetos == 'haiku'
             cc.find_library('network'),
             cc.find_library('bsd')]
 elif targetos == 'openbsd'
-  if not get_option('tcg').disabled() and target_dirs.length() > 0
+  if get_option('tcg') != 'disabled' and target_dirs.length() > 0
     # Disable OpenBSD W^X if available
     emulator_link_args = cc.get_supported_link_arguments('-Wl,-z,wxneeded')
   endif
@@ -241,7 +241,7 @@ if targetos == 'netbsd'
 endif
 
 tcg_arch = config_host['ARCH']
-if not get_option('tcg').disabled()
+if get_option('tcg') != 'disabled'
   if cpu not in supported_cpus
     if get_option('tcg_interpreter')
       warning('Unsupported CPU @0@, will use TCG with TCI (experimental and slow)'.format(cpu))
@@ -273,7 +273,11 @@ if not get_option('tcg').disabled()
                         language: ['c', 'cpp', 'objc'])
 
   accelerators += 'CONFIG_TCG'
-  config_host += { 'CONFIG_TCG': 'y' }
+  if get_option('tcg') == 'module'
+    config_host += { 'CONFIG_TCG': 'm' }
+  else
+    config_host += { 'CONFIG_TCG': 'y' }
+  endif
 endif
 
 if 'CONFIG_KVM' not in accelerators and get_option('kvm').enabled()
@@ -1306,19 +1310,20 @@ foreach target : target_dirs
   accel_kconfig = []
   foreach sym: accelerators
     if sym == 'CONFIG_TCG' or target in accelerator_targets.get(sym, [])
-      config_target += { sym: 'y' }
-      config_all += { sym: 'y' }
+      if sym == 'CONFIG_TCG' and get_option('tcg') == 'module'
+        config_target += { sym: 'm' }
+        config_all += { sym: 'm' }
+        accel_kconfig += [ sym + '=m' ]
+      else
+        config_target += { sym: 'y' }
+        config_all += { sym: 'y' }
+        accel_kconfig += [ sym + '=y' ]
+      endif
       if sym == 'CONFIG_TCG' and tcg_arch == 'tci'
         config_target += { 'CONFIG_TCG_INTERPRETER': 'y' }
       elif sym == 'CONFIG_XEN' and have_xen_pci_passthrough
         config_target += { 'CONFIG_XEN_PCI_PASSTHROUGH': 'y' }
       endif
-      if target in modular_tcg
-        config_target += { 'CONFIG_TCG_MODULAR': 'y' }
-      else
-        config_target += { 'CONFIG_TCG_BUILTIN': 'y' }
-      endif
-      accel_kconfig += [ sym + '=y' ]
     endif
   endforeach
   if accel_kconfig.length() == 0
@@ -2039,8 +2044,11 @@ subdir('tests/qtest/fuzz')
 
 # accel modules
 tcg_real_module_ss = ss.source_set()
-tcg_real_module_ss.add_all(when: 'CONFIG_TCG_MODULAR', if_true: tcg_module_ss)
-specific_ss.add_all(when: 'CONFIG_TCG_BUILTIN', if_true: tcg_module_ss)
+if get_option('tcg') == 'module'
+  tcg_real_module_ss.add_all(when: 'CONFIG_TCG_MODULAR', if_true: tcg_module_ss)
+else
+  specific_ss.add_all(when: 'CONFIG_TCG_BUILTIN', if_true: tcg_module_ss)
+endif
 target_modules += { 'accel' : { 'qtest': qtest_module_ss,
                                 'tcg': tcg_real_module_ss }}
 
@@ -2689,6 +2697,8 @@ summary_info += {'TCG support':       config_all.has_key('CONFIG_TCG')}
 if config_all.has_key('CONFIG_TCG')
   if get_option('tcg_interpreter')
     summary_info += {'TCG backend':   'TCI (TCG with bytecode interpreter, experimental and slow)'}
+  elif get_option('tcg') == 'module'
+    summary_info += {'TCG backend':   'module (@0@)'.format(cpu)}
   else
     summary_info += {'TCG backend':   'native (@0@)'.format(cpu)}
   endif
diff --git a/meson_options.txt b/meson_options.txt
index 3d304cac96..332dacd8ec 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -39,7 +39,8 @@ option('xen', type: 'feature', value: 'auto',
        description: 'Xen backend support')
 option('xen_pci_passthrough', type: 'feature', value: 'auto',
        description: 'Xen PCI passthrough support')
-option('tcg', type: 'feature', value: 'auto',
+option('tcg', type: 'combo', value: 'enabled',
+       choices: ['enabled', 'disabled', 'module'],
        description: 'TCG support')
 option('tcg_interpreter', type: 'boolean', value: false,
        description: 'TCG with bytecode interpreter (experimental and slow)')
diff --git a/scripts/minikconf.py b/scripts/minikconf.py
index bcd91015d3..2db0c3661f 100644
--- a/scripts/minikconf.py
+++ b/scripts/minikconf.py
@@ -323,6 +323,7 @@ def do_imply(self, var, symbol, cond=None):
 TOK_IF = 16;      TOKENS[TOK_IF] = '"if"';
 TOK_ID = 17;      TOKENS[TOK_ID] = 'identifier';
 TOK_EOF = 18;     TOKENS[TOK_EOF] = 'end of file';
+TOK_M = 19;       TOKENS[TOK_M] = '"m"';
 
 class KconfigParserError(Exception):
     def __init__(self, parser, msg, tok=None):
@@ -415,15 +416,18 @@ def do_include(self, include):
 
     # recursive descent parser -----
 
-    # y_or_n: Y | N
-    def parse_y_or_n(self):
+    # y_or_n_or_m: Y | N | M
+    def parse_y_or_n_or_m(self):
         if self.tok == TOK_Y:
             self.get_token()
             return True
         if self.tok == TOK_N:
             self.get_token()
             return False
-        raise KconfigParserError(self, 'Expected "y" or "n"')
+        if self.tok == TOK_M:
+            self.get_token()
+            return True
+        raise KconfigParserError(self, 'Expected "y", "n", or "m"')
 
     # var: ID
     def parse_var(self):
@@ -446,13 +450,13 @@ def parse_assignment_var(self):
         else:
             raise KconfigParserError(self, 'Expected identifier')
 
-    # assignment: var EQUAL y_or_n
+    # assignment: var EQUAL y_or_n_or_m
     def parse_assignment(self):
         var = self.parse_assignment_var()
         if self.tok != TOK_EQUAL:
             raise KconfigParserError(self, 'Expected "="')
         self.get_token()
-        self.data.do_assignment(var, self.parse_y_or_n())
+        self.data.do_assignment(var, self.parse_y_or_n_or_m())
 
     # primary: NOT primary
     #       | LPAREN expr RPAREN
@@ -505,7 +509,7 @@ def parse_condition(self):
     def parse_property(self, var):
         if self.tok == TOK_DEFAULT:
             self.get_token()
-            val = self.parse_y_or_n()
+            val = self.parse_y_or_n_or_m()
             cond = self.parse_condition()
             self.data.do_default(var, val, cond)
         elif self.tok == TOK_DEPENDS:
@@ -635,6 +639,8 @@ def scan_token(self):
             return TOK_Y
         elif self.tok == 'n' and self.check_keyword(""):
             return TOK_N
+        elif self.tok == 'm' and self.check_keyword(""):
+            return TOK_M
         elif (self.tok == 's' and self.check_keyword("ource")) or \
               self.tok == 'i' and self.check_keyword("nclude"):
             # source FILENAME
@@ -690,10 +696,10 @@ def scan_token(self):
     parser = KconfigParser(data)
     external_vars = set()
     for arg in argv[3:]:
-        m = re.match(r'^(CONFIG_[A-Z0-9_]+)=([yn]?)$', arg)
+        m = re.match(r'^(CONFIG_[A-Z0-9_]+)=([ynm]?)$', arg)
         if m is not None:
             name, value = m.groups()
-            parser.do_assignment(name, value == 'y')
+            parser.do_assignment(name, value == 'y' or value == 'm')
             external_vars.add(name[7:])
         else:
             fp = open(arg, 'rt', encoding='utf-8')
diff --git a/tests/meson.build b/tests/meson.build
index 55a7b08275..d3800989ee 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -80,7 +80,7 @@ if 'CONFIG_TCG' in config_all
   subdir('fp')
 endif
 
-if not get_option('tcg').disabled()
+if get_option('tcg') != 'disabled'
   if 'CONFIG_PLUGIN' in config_host
     subdir('plugin')
   endif
-- 
2.32.0


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]