From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2C382A0547; Tue, 9 Feb 2021 12:51:08 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B64CF160722; Tue, 9 Feb 2021 12:51:07 +0100 (CET) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mails.dpdk.org (Postfix) with ESMTP id 571F240147 for ; Tue, 9 Feb 2021 12:51:06 +0100 (CET) IronPort-SDR: 71SD7dApLEbYDAStuhPyjE5PAyhxmulJfFosf97ELoTjlj9dnhheWJWVyl0+arYq+cBvTQnH3+ yytqXv5nAnvg== X-IronPort-AV: E=McAfee;i="6000,8403,9889"; a="245935557" X-IronPort-AV: E=Sophos;i="5.81,164,1610438400"; d="scan'208";a="245935557" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Feb 2021 03:51:04 -0800 IronPort-SDR: JZg1SzxT0ktXdH+rzH8G4SPIktSaAqGovaWr+kENh1vt2ahBOy89pCqNf1b8/dHueihnKdcjqW BTbm7Dm1Dt3g== X-IronPort-AV: E=Sophos;i="5.81,164,1610438400"; d="scan'208";a="378878663" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.24.112]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 09 Feb 2021 03:51:02 -0800 Date: Tue, 9 Feb 2021 11:50:59 +0000 From: Bruce Richardson To: Juraj =?utf-8?Q?Linke=C5=A1?= Cc: thomas@monjalon.net, Ruifeng.Wang@arm.com, Honnappa.Nagarahalli@arm.com, jerinjacobk@gmail.com, hemant.agrawal@nxp.com, ferruh.yigit@intel.com, aboyer@pensando.io, dev@dpdk.org Message-ID: <20210209115059.GA1208@bricha3-MOBL.ger.corp.intel.com> References: <1612537472-18192-1-git-send-email-juraj.linkes@pantheon.tech> <1612860425-27646-1-git-send-email-juraj.linkes@pantheon.tech> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1612860425-27646-1-git-send-email-juraj.linkes@pantheon.tech> Subject: Re: [dpdk-dev] [PATCH v5] build: kni cross-compilation support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Tue, Feb 09, 2021 at 09:47:05AM +0100, Juraj Linkeš wrote: > The kni linux module is using a custom target for building, which > doesn't take into account any cross compilation arguments. The arguments > in question are ARCH, CROSS_COMPILE (for gcc, clang) and CC, LD (for > clang). Get those from the cross file and pass them to the custom > target. > > The user supplied path may not contain the 'build' directory, such as > when using cross-compiled headers, so only append that in the default > case (when no path is supplied in native builds) and use the unmodified > path from the user otherwise. Also modify the install path accordingly. > > Signed-off-by: Juraj Linkeš Reviewed-by: Bruce Richardson Couple of small comments inline below. > --- > kernel/linux/kni/meson.build | 8 +-- > kernel/linux/meson.build | 94 +++++++++++++++++++++++++++++++----- > meson_options.txt | 2 +- > 3 files changed, 86 insertions(+), 18 deletions(-) > > diff --git a/kernel/linux/kni/meson.build b/kernel/linux/kni/meson.build > index 07e0c9dae7..46b71c7418 100644 > --- a/kernel/linux/kni/meson.build > +++ b/kernel/linux/kni/meson.build > @@ -13,7 +13,7 @@ kni_sources = files( > custom_target('rte_kni', > input: kni_sources, > output: 'rte_kni.ko', > - command: ['make', '-j4', '-C', kernel_dir + '/build', > + command: ['make', '-j4', '-C', kernel_build_dir, > 'M=' + meson.current_build_dir(), > 'src=' + meson.current_source_dir(), > 'MODULE_CFLAGS=-include ' + meson.source_root() + '/config/rte_config.h' + > @@ -21,8 +21,8 @@ custom_target('rte_kni', > ' -I' + meson.source_root() + '/lib/librte_kni' + > ' -I' + meson.build_root() + > ' -I' + meson.current_source_dir(), > - 'modules'], > + 'modules'] + cross_args, > depends: kni_mkfile, > - install: true, > - install_dir: kernel_dir + '/extra/dpdk', > + install: install, > + install_dir: kernel_install_dir, > build_by_default: get_option('enable_kmods')) > diff --git a/kernel/linux/meson.build b/kernel/linux/meson.build > index 5c864a4653..67c1cbd7e0 100644 > --- a/kernel/linux/meson.build > +++ b/kernel/linux/meson.build > @@ -3,25 +3,93 @@ > > subdirs = ['kni'] > > -# if we are cross-compiling we need kernel_dir specified > -if get_option('kernel_dir') == '' and meson.is_cross_build() > - error('Need "kernel_dir" option for kmod compilation when cross-compiling') > -endif > +kernel_build_dir = get_option('kernel_dir') > +kernel_install_dir = '' > +install = not meson.is_cross_build() > +cross_args = [] > > -kernel_dir = get_option('kernel_dir') > -if kernel_dir == '' > - # use default path for native builds > +if not meson.is_cross_build() > + # native build > kernel_version = run_command('uname', '-r').stdout().strip() > - kernel_dir = '/lib/modules/' + kernel_version > + kernel_install_dir = '/lib/modules/' + kernel_version + '/extra/dpdk' > + if kernel_build_dir == '' > + # use default path for native builds > + kernel_build_dir = '/lib/modules/' + kernel_version + '/build' > + endif > + > + # test running make in kernel directory, using "make kernelversion" > + make_returncode = run_command('make', '-sC', kernel_build_dir, > + 'kernelversion').returncode() > + if make_returncode != 0 > + # backwards compatibility: > + # the headers could still be in the 'build' subdir > + if not kernel_build_dir.endswith('build') and not kernel_build_dir.endswith('build/') > + kernel_build_dir = join_paths(kernel_build_dir, 'build') > + make_returncode = run_command('make', '-sC', kernel_build_dir, > + 'kernelversion').returncode() > + if make_returncode == 0 > + warning('Deprecation notice: Specifying kernel_dir ' + > + 'without the build dir will be deprecated in the future.') > + endif I don't really think we need to deprecate this, since this should adequately support both options. Therefore, I suggest just dropping these last 4 lines and accepting both options. > + endif > + endif > + > + if make_returncode != 0 > + error('Cannot compile kernel modules as requested - are kernel headers installed?') > + endif > + > + # DO ACTUAL MODULE BUILDING > + foreach d:subdirs > + subdir(d) > + endforeach > + > + subdir_done() > endif > > -# test running make in kernel directory, using "make kernelversion" > -make_returncode = run_command('make', '-sC', kernel_dir + '/build', > - 'kernelversion').returncode() > -if make_returncode != 0 > - error('Cannot compile kernel modules as requested - are kernel headers installed?') > +# cross build > +# if we are cross-compiling we need kernel_build_dir specified > +if kernel_build_dir == '' > + error('Need "kernel_dir" option for kmod compilation when cross-compiling') > +endif > +cross_compiler = find_program('c').path() > +if cross_compiler.endswith('gcc') > + cross_prefix = run_command([py3, '-c', 'print("' + cross_compiler + '"[:-3])']).stdout().strip() > +elif cross_compiler.endswith('clang') > + cross_prefix = '' > + found_target = false > + # search for '-target' and use the arg that follows > + # (i.e. the value of '-target') as cross_prefix > + foreach cross_c_arg : meson.get_cross_property('c_args') > + if found_target and cross_prefix == '' > + cross_prefix = cross_c_arg > + endif > + if cross_c_arg == '-target' > + found_target = true > + endif > + endforeach > + if cross_prefix == '' > + error('Didn\'t find -target and its value in' + > + ' c_args in input cross-file.') > + endif > + linker = 'lld' > + foreach cross_c_link_arg : meson.get_cross_property('c_link_args') > + if cross_c_link_arg.startswith('-fuse-ld') > + linker = cross_c_link_arg.split('=')[1] > + endif > + endforeach > + cross_args += ['CC=@0@'.format(cross_compiler), 'LD=ld.@0@'.format(linker)] > +else > + error('Unsupported cross compiler: @0@'.format(cross_compiler)) > endif > > +cross_arch = host_machine.cpu_family() > +if host_machine.cpu_family() == 'aarch64' > + cross_arch = 'arm64' > +endif > + > +cross_args += ['ARCH=@0@'.format(cross_arch), > + 'CROSS_COMPILE=@0@'.format(cross_prefix)] > + > # DO ACTUAL MODULE BUILDING > foreach d:subdirs > subdir(d) > diff --git a/meson_options.txt b/meson_options.txt > index 6eff62e47d..de44d17c02 100644 > --- a/meson_options.txt > +++ b/meson_options.txt > @@ -19,7 +19,7 @@ option('ibverbs_link', type: 'combo', choices : ['static', 'shared', 'dlopen'], > option('include_subdir_arch', type: 'string', value: '', > description: 'subdirectory where to install arch-dependent headers') > option('kernel_dir', type: 'string', value: '', > - description: 'Path to the kernel for building kernel modules. Headers must be in $kernel_dir/build. Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.') > + description: 'Path to the kernel for building kernel modules. Headers must be in $kernel_dir. Modules will be installed in /lib/modules/$(uname -r)/extra/dpdk for native builds.') While correct, the last sentence isn't really relevant to the build option, it's more an additional note, and it seems a bit awkward. Maybe it can be shortened by referencing "/lib/modules" rather than giving the full path. Alternatively, maybe just putting the details in the documentation is a better plan, and leaving the comment here as just "path to the kernel for building kernel modules". > option('machine', type: 'string', value: 'native', > description: 'set the target machine type') > option('max_ethports', type: 'integer', value: 32, > -- > 2.20.1 >