Re: [RFC v2 1/6] build: Add configure options for native calls

["Alex Bennée" <alex.bennee@linaro.org>]

Yeqi Fu <fufuyqqqqqq@gmail.com> writes:

> Signed-off-by: Yeqi Fu <fufuyqqqqqq@gmail.com>
> ---
>  Makefile                            |  4 +++
>  common-user/native/Makefile.include |  9 ++++++
>  common-user/native/Makefile.target  | 22 +++++++++++++
>  configure                           | 50 +++++++++++++++++++++++++++++
>  docs/devel/build-system.rst         |  4 +++
>  meson.build                         |  8 +++++
>  meson_options.txt                   |  2 ++
>  scripts/meson-buildoptions.sh       |  4 +++
>  8 files changed, 103 insertions(+)
>  create mode 100644 common-user/native/Makefile.include
>  create mode 100644 common-user/native/Makefile.target
>
> diff --git a/Makefile b/Makefile
> index 3c7d67142f..923da109bf 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -185,6 +185,10 @@ SUBDIR_MAKEFLAGS=$(if $(V),,--no-print-directory --quiet)
>  
>  include $(SRC_PATH)/tests/Makefile.include
>  
> +ifeq ($(CONFIG_USER_NATIVE),y)
> +	include $(SRC_PATH)/common-user/native/Makefile.include
> +endif
> +
>  all: recurse-all
>  
>  ROMS_RULES=$(foreach t, all clean distclean, $(addsuffix /$(t), $(ROMS)))
> diff --git a/common-user/native/Makefile.include b/common-user/native/Makefile.include
> new file mode 100644
> index 0000000000..40d20bcd4c
> --- /dev/null
> +++ b/common-user/native/Makefile.include
> @@ -0,0 +1,9 @@
> +.PHONY: build-native
> +build-native: $(NATIVE_TARGETS:%=build-native-library-%)
> +$(NATIVE_TARGETS:%=build-native-library-%): build-native-library-%:
> +	$(call quiet-command, \
> +	    $(MAKE) -C common-user/native/$* $(SUBDIR_MAKEFLAGS), \
> +	"BUILD","$* native library")
> +# endif
> +
> +all: build-native

I think it would be better if we could add the targets via meson and let
it deal with the multiple versions. I will defer to Paolo on how to do
this though.


> diff --git a/common-user/native/Makefile.target b/common-user/native/Makefile.target
> new file mode 100644
> index 0000000000..1038367b37
> --- /dev/null
> +++ b/common-user/native/Makefile.target
> @@ -0,0 +1,22 @@
> +# -*- Mode: makefile -*-
> +#
> +# Library for native calls 
> +#
> +
> +all:
> +-include ../config-host.mak
> +-include config-target.mak
> +
> +CFLAGS+=-I$(SRC_PATH)/include -O1 -fPIC -shared -fno-stack-protector
> +LDFLAGS+=
> +
> +SRC = $(SRC_PATH)/common-user/native/libnative.c
> +TARGET = libnative.so
> +
> +all: $(TARGET)
> +
> +$(TARGET): $(SRC)
> +	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
> +
> +clean:
> +	rm -f $(TARGET)
> diff --git a/configure b/configure
> index 2a556d14c9..cc94d10c98 100755
> --- a/configure
> +++ b/configure
> @@ -275,6 +275,7 @@ use_containers="yes"
>  gdb_bin=$(command -v "gdb-multiarch" || command -v "gdb")
>  gdb_arches=""
>  werror=""
> +user_native_call="disabled"
>  
>  # Don't accept a target_list environment variable.
>  unset target_list
> @@ -787,6 +788,10 @@ for opt do
>    ;;
>    --disable-vfio-user-server) vfio_user_server="disabled"
>    ;;
> +  --enable-user-native-call) user_native_call="enabled"
> +  ;;
> +  --disable-user-native-call) user_native_call="disabled"
> +  ;;

I'm not sure it's worth the configuration control here. We can embed if
a given frontend has support for native calls in:

  config/targets/FOO-linux-user.mak

and simply add the symbol when each front end is enabled.

>    # everything else has the same name in configure and meson
>    --*) meson_option_parse "$opt" "$optarg"
>    ;;
> @@ -1898,6 +1903,50 @@ if test "$tcg" = "enabled"; then
>  fi
>  )
>  
> +# common-user/native configuration
> +native_flag_i386="-DTARGET_I386"
> +native_flag_x86_64="-DTARGET_X86_64"
> +native_flag_mips="-DTARGET_MIPS"
> +native_flag_mips64="-DTARGET_MIPS64"
> +native_flag_arm="-DTARGET_ARM"
> +native_flag_aarch64="-DTARGET_AARCH64"

As we have target names already in the per-target configs we could use
that instead and build the cflags there.

> +
> +(config_host_mak=common-user/native/config-host.mak
> +mkdir -p common-user/native
> +echo "# Automatically generated by configure - do not modify" > $config_host_mak
> +echo "SRC_PATH=$source_path" >> $config_host_mak
> +echo "HOST_CC=$host_cc" >> $config_host_mak
> +
> +native_targets=
> +for target in $target_list; do
> +  arch=${target%%-*}
> +
> +  case $target in
> +    *-linux-user|*-bsd-user)
> +    if probe_target_compiler $target || test -n "$container_image"; then
> +        mkdir -p "common-user/native/$target"
> +        config_target_mak=common-user/native/$target/config-target.mak
> +        ln -sf "$source_path/common-user/native/Makefile.target" "common-user/native/$target/Makefile"
> +        echo "# Automatically generated by configure - do not modify" > "$config_target_mak"
> +        echo "TARGET_NAME=$arch" >> "$config_target_mak"
> +        echo "TARGET=$target" >> "$config_target_mak"
> +        eval "target_native_flag=\${native_flag_$target_arch}"
> +        target_cflags="$target_cflags $target_native_flag"
> +        write_target_makefile "build-native-library-$target" >> "$config_target_mak"
> +        native_targets="$native_targets $target"
> +    fi
> +  ;;
> +  esac
> +done

This is basically replicating what we already have in
tests/tcg/FOO-linux-user/config-target.mak. I would suggest moving those
into a common location ($BUILD/targets/foo/compiler.mak) and then fixing
up TCG tests to use the new location. When you add the native libs you
can use the same configs.

> +
> +# if native enabled
> +if test "$user_native_call" = "enabled"; then
> +    echo "CONFIG_USER_NATIVE=y" >> config-host.mak
> +    echo "NATIVE_TARGETS=$native_targets" >> config-host.mak
> +    
> +fi
> +)
> +

see above about putting CONFIG_USER_NATIVE directly into the target mak fragments.

>  if test "$skip_meson" = no; then
>    cross="config-meson.cross.new"
>    meson_quote() {
> @@ -1980,6 +2029,7 @@ if test "$skip_meson" = no; then
>    test "$smbd" != '' && meson_option_add "-Dsmbd=$smbd"
>    test "$tcg" != enabled && meson_option_add "-Dtcg=$tcg"
>    test "$vfio_user_server" != auto && meson_option_add "-Dvfio_user_server=$vfio_user_server"
> +  test "$user_native_call" != auto && meson_option_add
>    "-Duser_native_call=$user_native_call"

and dropping this.

>    run_meson() {
>      NINJA=$ninja $meson setup --prefix "$prefix" "$@" $cross_arg "$PWD" "$source_path"
>    }
> diff --git a/docs/devel/build-system.rst b/docs/devel/build-system.rst
> index 551c5a5ac0..05cfa8a21a 100644
> --- a/docs/devel/build-system.rst
> +++ b/docs/devel/build-system.rst
> @@ -494,6 +494,10 @@ Built by configure:
>    Configuration variables used to build the firmware and TCG tests,
>    including paths to cross compilation toolchains.
>  
> +``common-user/native/config-host.mak``, ``common-user/native/*/config-target.mak``
> +  Configuration variables used to build the native call libraries
> +  including paths to cross compilation toolchains.
> +

Not needed if we re-use the TCG stuff. But remember to update the
section above when moving them.

>  ``pyvenv``
>  
>    A Python virtual environment that is used for all Python code running
> diff --git a/meson.build b/meson.build
> index 0a5cdefd4d..04e99a4f25 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2012,6 +2012,11 @@ have_virtfs_proxy_helper = get_option('virtfs_proxy_helper') \
>      .require(libcap_ng.found(), error_message: 'the virtfs proxy helper requires libcap-ng') \
>      .allowed()
>  
> +have_user_native_call = get_option('user_native_call') \
> +    .require(have_user, error_message: 'user_native_call requires user') \
> +    .require(targetos == 'linux', error_message: 'user_native_call requires Linux') \
> +    .allowed()
> +
>  if get_option('block_drv_ro_whitelist') == ''
>    config_host_data.set('CONFIG_BDRV_RO_WHITELIST', '')
>  else
> @@ -2853,6 +2858,9 @@ foreach target : target_dirs
>        error('Target @0@ is only available on a Linux host'.format(target))
>      endif
>      config_target += { 'CONFIG_LINUX_USER': 'y' }
> +    if have_user_native_call
> +      config_target += { 'CONFIG_USER_NATIVE_CALL': 'y' }
> +    endif

Not needed? Isn't CONFIG_USER_NATIVE an equivalent test?

>    elif target.endswith('bsd-user')
>      if 'CONFIG_BSD' not in config_host
>        if default_targets
> diff --git a/meson_options.txt b/meson_options.txt
> index 90237389e2..57035e02f5 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -352,3 +352,5 @@ option('slirp_smbd', type : 'feature', value : 'auto',
>  
>  option('hexagon_idef_parser', type : 'boolean', value : true,
>         description: 'use idef-parser to automatically generate TCG code for the Hexagon frontend')
> +option('user_native_call', type : 'feature', value : 'disabled',
> +       description: 'native bypass for library calls in user mode only')
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index 5714fd93d9..9eda1898d6 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -173,6 +173,8 @@ meson_options_help() {
>    printf "%s\n" '  tpm             TPM support'
>    printf "%s\n" '  u2f             U2F emulation support'
>    printf "%s\n" '  usb-redir       libusbredir support'
> +  printf "%s\n" '  user-native-call'
> +  printf "%s\n" '                  native bypass for library calls in user mode only'
>    printf "%s\n" '  vde             vde network backend support'
>    printf "%s\n" '  vdi             vdi image format support'
>    printf "%s\n" '  vduse-blk-export'
> @@ -472,6 +474,8 @@ _meson_option_parse() {
>      --disable-u2f) printf "%s" -Du2f=disabled ;;
>      --enable-usb-redir) printf "%s" -Dusb_redir=enabled ;;
>      --disable-usb-redir) printf "%s" -Dusb_redir=disabled ;;
> +    --enable-user-native-call) printf "%s" -Duser_native_call=enabled ;;
> +    --disable-user-native-call) printf "%s" -Duser_native_call=disabled ;;
>      --enable-vde) printf "%s" -Dvde=enabled ;;
>      --disable-vde) printf "%s" -Dvde=disabled ;;
>      --enable-vdi) printf "%s" -Dvdi=enabled ;;

And then you can drop this.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro