[RFC 0/2] Option to build native TCG with --enable-modules

[RFC 0/2] Option to build native TCG with --enable-modules

[Jose R. Ziviani]

Hello!

I'm sending this simple patchset based on a patch still on review[1]
just to understand if it's something that makes sense to the community.

If so, I think it could be included in Gerd's patchset.

Thank you!

[1] https://patchwork.kernel.org/project/qemu-devel/list/?series=506379

Jose R. Ziviani (2):
  modules: Option to build native TCG with --enable-modules
  modules: Fix warning in module_arch documentation

 configure             | 12 ++++++++++--
 include/qemu/module.h |  2 +-
 meson.build           |  7 ++++++-
 meson_options.txt     |  2 ++
 4 files changed, 19 insertions(+), 4 deletions(-)

-- 
2.32.0

[RFC 1/2] modules: Option to build native TCG with --enable-modules

[Jose R. Ziviani]

Adds an option (--enable-tcg-builtin) to build TCG natively when
--enable-modules argument is passed to the build system. It gives
the opportunity to have the accelerator built-in and still take
advantage of the new modular system.

Signed-off-by: Jose R. Ziviani <jziviani@suse.de>
---
 configure         | 12 ++++++++++--
 meson.build       |  7 ++++++-
 meson_options.txt |  2 ++
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 38704b4e11..add8cf4970 100755
--- a/configure
+++ b/configure
@@ -344,6 +344,7 @@ tsan="no"
 fortify_source="$default_feature"
 strip_opt="yes"
 tcg_interpreter="false"
+tcg_builtin="false"
 bigendian="no"
 mingw32="no"
 gcov="no"
@@ -1107,6 +1108,8 @@ for opt do
   ;;
   --enable-tcg) tcg="enabled"
   ;;
+  --enable-tcg-builtin) tcg_builtin="true"
+  ;;
   --disable-malloc-trim) malloc_trim="disabled"
   ;;
   --enable-malloc-trim) malloc_trim="enabled"
@@ -1792,6 +1795,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)
+  --enable-tcg-builtin     force TCG builtin even with --enable-modules
   --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 +2292,11 @@ if test "$solaris" = "yes" ; then
   fi
 fi
 
-if test "$tcg" = "enabled"; then
+if test "$tcg" = "disabled"; then
+    debug_tcg="no"
+    tcg_interpreter="false"
+    tcg_builtin="false"
+else
     git_submodules="$git_submodules tests/fp/berkeley-testfloat-3"
     git_submodules="$git_submodules tests/fp/berkeley-softfloat-3"
 fi
@@ -6449,7 +6457,7 @@ if test "$skip_meson" = no; then
         -Dvhost_user_blk_server=$vhost_user_blk_server -Dmultiprocess=$multiprocess \
         -Dfuse=$fuse -Dfuse_lseek=$fuse_lseek -Dguest_agent_msi=$guest_agent_msi -Dbpf=$bpf\
         $(if test "$default_features" = no; then echo "-Dauto_features=disabled"; fi) \
-	-Dtcg_interpreter=$tcg_interpreter \
+        -Dtcg_interpreter=$tcg_interpreter -Dtcg_builtin=$tcg_builtin \
         $cross_arg \
         "$PWD" "$source_path"
 
diff --git a/meson.build b/meson.build
index fe58793549..2d72b8cc06 100644
--- a/meson.build
+++ b/meson.build
@@ -93,6 +93,9 @@ if cpu in ['x86', 'x86_64']
 endif
 
 modular_tcg = ['i386-softmmu', 'x86_64-softmmu']
+is_tcg_modular = config_host.has_key('CONFIG_MODULES') \
+                 and get_option('tcg').enabled() \
+                 and not get_option('tcg_builtin')
 
 edk2_targets = [ 'arm-softmmu', 'aarch64-softmmu', 'i386-softmmu', 'x86_64-softmmu' ]
 install_edk2_blobs = false
@@ -1313,7 +1316,7 @@ foreach target : target_dirs
       elif sym == 'CONFIG_XEN' and have_xen_pci_passthrough
         config_target += { 'CONFIG_XEN_PCI_PASSTHROUGH': 'y' }
       endif
-      if target in modular_tcg
+      if target in modular_tcg and is_tcg_modular
         config_target += { 'CONFIG_TCG_MODULAR': 'y' }
       else
         config_target += { 'CONFIG_TCG_BUILTIN': 'y' }
@@ -2698,6 +2701,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 is_tcg_modular
+    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..fd9f92b333 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -43,6 +43,8 @@ option('tcg', type: 'feature', value: 'auto',
        description: 'TCG support')
 option('tcg_interpreter', type: 'boolean', value: false,
        description: 'TCG with bytecode interpreter (experimental and slow)')
+option('tcg_builtin', type: 'boolean', value: 'false',
+       description: 'Force TCG builtin')
 option('cfi', type: 'boolean', value: 'false',
        description: 'Control-Flow Integrity (CFI)')
 option('cfi_debug', type: 'boolean', value: 'false',
-- 
2.32.0

Re: [RFC 1/2] modules: Option to build native TCG with --enable-modules

[Gerd Hoffmann]

On Tue, Jun 29, 2021 at 06:28:18PM -0300, Jose R. Ziviani wrote:
> Adds an option (--enable-tcg-builtin) to build TCG natively when
> --enable-modules argument is passed to the build system. It gives
> the opportunity to have the accelerator built-in and still take
> advantage of the new modular system.

I think we should not special-case tcg here.  Either allow setting =y
for all modules with all the consequences this has for maintainance and
testing, or leave things as-is.

See also recent discussions kicked by Claudio Fontana.

take care,
  Gerd

PS: just back from vacation, wading backwards through
    my mail backlog ...).

Re: [RFC 1/2] modules: Option to build native TCG with --enable-modules

[Claudio Fontana]

On 7/21/21 12:59 PM, Gerd Hoffmann wrote:
> On Tue, Jun 29, 2021 at 06:28:18PM -0300, Jose R. Ziviani wrote:
>> Adds an option (--enable-tcg-builtin) to build TCG natively when
>> --enable-modules argument is passed to the build system. It gives
>> the opportunity to have the accelerator built-in and still take
>> advantage of the new modular system.
> 
> I think we should not special-case tcg here.  Either allow setting =y
> for all modules with all the consequences this has for maintainance and

agreed;

> testing, or leave things as-is.> 
> See also recent discussions kicked by Claudio Fontana.

maybe this "all-y" for modules vs "all-m" could be a good step in the right direction,
with arbitrary -m , -y configuration being a potential next step if ppl find it a useful extension?

Hmm..

Ciao,

Claudio

> 
> take care,
>   Gerd
> 
> PS: just back from vacation, wading backwards through
>     my mail backlog ...).
> 

[RFC 2/2] modules: Fix warning in module_arch documentation

[Jose R. Ziviani]

Fixes a small issue in the module_arch documentation that
caused the build system to complain:
module.h:127: warning: Function parameter or member
              'name' not described in 'module_arch'
module.h:127: warning: Excess function parameter
              'arch' description in 'module_arch'

Signed-off-by: Jose R. Ziviani <jziviani@suse.de>
---
 include/qemu/module.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qemu/module.h b/include/qemu/module.h
index 8bc80535a4..456e190a55 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -115,7 +115,7 @@ void module_allow_arch(const char *arch);
 /**
  * module_arch
  *
- * @arch: target architecture
+ * @name: module name
  *
  * This module is for target architecture @arch.
  *
-- 
2.32.0