From: Bruce Richardson <bruce.richardson@intel.com>
To: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
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
Subject: Re: [dpdk-dev] [RFC PATCH v2] build: kni cross-compilation support
Date: Thu, 4 Feb 2021 17:33:52 +0000 [thread overview]
Message-ID: <20210204173352.GE1712@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <1612432301-14961-1-git-send-email-juraj.linkes@pantheon.tech>
On Thu, Feb 04, 2021 at 10:51:41AM +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š <juraj.linkes@pantheon.tech>
Some comments inline below.
/Bruce
> ---
> kernel/linux/kni/meson.build | 8 +++---
> kernel/linux/meson.build | 56 ++++++++++++++++++++++++++++++++----
> 2 files changed, 55 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/linux/kni/meson.build b/kernel/linux/kni/meson.build
> index 07e0c9dae7..bb7123354f 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_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: install_dir,
> build_by_default: get_option('enable_kmods'))
rather than "kernel_dir" and "install_dir" can we rename these to
"kernel_build_dir" and "kernel_install_dir" for clarity.
> diff --git a/kernel/linux/meson.build b/kernel/linux/meson.build
> index 5c864a4653..74097299bb 100644
> --- a/kernel/linux/meson.build
> +++ b/kernel/linux/meson.build
> @@ -3,20 +3,66 @@
>
> subdirs = ['kni']
>
> +kernel_version = run_command('uname', '-r').stdout().strip()
Rename to "host_kernel_version" and probably should be only queried in the
native build case.
> +cross_args = []
> # 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')
> +if meson.is_cross_build()
> + if get_option('kernel_dir') == ''
> + error('Need "kernel_dir" option for kmod compilation when cross-compiling')
> + else
> + install_dir = ''
> + install = false
I think these should be defined and initialized further up the file,
outside the conditional block.
> + 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
> + if host_machine.cpu_family() == 'aarch64'
> + cross_arch = 'arm64'
> + else
> + cross_arch = build_machine.cpu_family()
> + endif
> + cross_args += ['ARCH=@0@'.format(cross_arch),
> + 'CROSS_COMPILE=@0@'.format(cross_prefix)]
> + endif
> +else
> + install_dir = '/lib/modules/' + kernel_version + '/extra/dpdk'
> + install = true
> endif
>
The block for cross-compiling is fairly large and complex, so I'm wondering
how we can simplify things a bit. If we had multiple kernel modules I'd
suggest splitting thing up into a native and cross-build subdirectories to
get the build info, but that seems like overkill here. Instead I wonder if
it would work better to handle all the native case initially in a hopefully
simpler "if block" and then do subdir_done(), leaving everything else to be
the cross-compilation recipe and saving at least one level of indentation.
Also, throughout the block, anywhere you have an "error()" call you can use
"endif" instead of "else" and save more indentation space.
> kernel_dir = get_option('kernel_dir')
> if kernel_dir == ''
> # use default path for native builds
> - kernel_version = run_command('uname', '-r').stdout().strip()
> - kernel_dir = '/lib/modules/' + kernel_version
> + kernel_dir = '/lib/modules/' + kernel_version + '/build'
> endif
>
> # test running make in kernel directory, using "make kernelversion"
> -make_returncode = run_command('make', '-sC', kernel_dir + '/build',
> +make_returncode = run_command('make', '-sC', kernel_dir,
> 'kernelversion').returncode()
> if make_returncode != 0
> error('Cannot compile kernel modules as requested - are kernel headers installed?')
> --
> 2.20.1
>
next prev parent reply other threads:[~2021-02-04 17:34 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-29 10:29 [dpdk-dev] [RFC PATCH v1] build: kni gcc " Juraj Linkeš
2021-01-29 11:43 ` Bruce Richardson
2021-01-29 12:33 ` Juraj Linkeš
2021-01-29 13:51 ` Bruce Richardson
2021-01-29 14:36 ` Juraj Linkeš
2021-01-29 14:42 ` Bruce Richardson
2021-01-29 14:47 ` Juraj Linkeš
2021-01-29 15:01 ` Bruce Richardson
2021-01-29 15:17 ` Juraj Linkeš
2021-01-29 15:39 ` Bruce Richardson
2021-02-01 7:48 ` Juraj Linkeš
2021-02-04 9:51 ` [dpdk-dev] [RFC PATCH v2] build: kni " Juraj Linkeš
2021-02-04 17:33 ` Bruce Richardson [this message]
2021-02-05 9:26 ` Juraj Linkeš
2021-02-05 9:38 ` Bruce Richardson
2021-02-05 9:44 ` Thomas Monjalon
2021-02-05 9:42 ` Bruce Richardson
2021-02-05 14:46 ` [dpdk-dev] [RFC PATCH v3] " Juraj Linkeš
2021-02-05 14:52 ` Bruce Richardson
2021-02-05 15:02 ` Juraj Linkeš
2021-02-05 15:04 ` [dpdk-dev] [RFC PATCH v4] " Juraj Linkeš
2021-02-05 15:27 ` Bruce Richardson
2021-02-08 10:17 ` Juraj Linkeš
2021-02-08 10:26 ` Bruce Richardson
2021-02-08 10:56 ` Thomas Monjalon
2021-02-08 11:05 ` Bruce Richardson
2021-02-08 11:21 ` Thomas Monjalon
2021-02-08 11:45 ` Bruce Richardson
2021-02-08 17:23 ` Thomas Monjalon
2021-02-09 8:47 ` [dpdk-dev] [PATCH v5] " Juraj Linkeš
2021-02-09 11:50 ` Bruce Richardson
2021-02-09 12:07 ` Juraj Linkeš
2021-02-09 12:39 ` Bruce Richardson
2021-02-11 12:59 ` [dpdk-dev] [PATCH v6] " Juraj Linkeš
2021-03-09 8:47 ` Juraj Linkeš
2021-03-09 16:26 ` Andrew Boyer
2021-03-15 22:45 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210204173352.GE1712@bricha3-MOBL.ger.corp.intel.com \
--to=bruce.richardson@intel.com \
--cc=Honnappa.Nagarahalli@arm.com \
--cc=Ruifeng.Wang@arm.com \
--cc=aboyer@pensando.io \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=hemant.agrawal@nxp.com \
--cc=jerinjacobk@gmail.com \
--cc=juraj.linkes@pantheon.tech \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).