Re: [dpdk-dev] [RFC PATCH v1] build: kni gcc cross-compilation support

["Juraj Linkeš" <juraj.linkes@pantheon.tech>]

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, January 29, 2021 12:44 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: thomas@monjalon.net; Ruifeng.Wang@arm.com; jerinjacobk@gmail.com;
> hemant.agrawal@nxp.com; ferruh.yigit@intel.com; aboyer@pensando.io;
> dev@dpdk.org
> Subject: Re: [RFC PATCH v1] build: kni gcc cross-compilation support
> 
> On Fri, Jan 29, 2021 at 11:29:19AM +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 and CROSS_COMPILE. 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.
> >
> > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > ---
> >  kernel/linux/kni/meson.build |  4 ++--
> >  kernel/linux/meson.build     | 33 +++++++++++++++++++++++++++++----
> >  2 files changed, 31 insertions(+), 6 deletions(-)
> >
> > diff --git a/kernel/linux/kni/meson.build
> > b/kernel/linux/kni/meson.build index 07e0c9dae..0fbf52c93 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,7 +21,7 @@ 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', diff --git
> > a/kernel/linux/meson.build b/kernel/linux/meson.build index
> > 5c864a465..57ed9bc48 100644
> > --- a/kernel/linux/meson.build
> > +++ b/kernel/linux/meson.build
> > @@ -3,20 +3,45 @@
> >
> >  subdirs = ['kni']
> >
> > +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
> > +		cross_compiler = find_program('c').path()
> > +		if cross_compiler.endswith('gcc')
> > +			cross_prefix = ''
> > +			# remove the 'gcc' suffix
> > +			# meson doesn't support removing elements from an
> array
> > +			# nor does it support slicing, so do it on our own
> > +			foreach element : cross_compiler.split('-')
> > +				if element != 'gcc'
> > +					cross_prefix += '@0@-
> '.format(element)
> > +				endif
> > +			endforeach
> > +		else
> > +			error('Unsupported cross compiler:
> @0@'.format(cross_compiler))
> > +		endif
> 
> Rather than splitting manually, might it be better to just define a new property in
> the cross-file to hold the prefix?

That would by one more unnecessary input, so I don't like that.

> Alternatively, rather than meson looping, why
> not just use "run_command" to use shell or python to do the job, e.g. [untested]
> 
> run_command([py3, '-c',
> 	'print("-".join("' + cross_compiler + '".split("-")[:-1]))')
> 
> run_command('bash', '-c',
> 	'echo ' + cross_compiler + ' | sed "s/gcc$//"')
> 

Since there isn't a better way to do this in Meson, it makes sense to use an external tool.
On top of that, this would save lines and wouldn't need as many code comments. I'll change it.

> > +		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
> >  endif
> >
> >  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
> 
> The reason we don't keep the "build" off the kernel_dir is to ensure that the
> kernel modules install to the correct place. With this change the modules will go
> in "/lib/modules/<version>/build/extra/dpdk", rather than
> "/lib/modules/<version>/extra/dpdk".
> 

Ah, I see. The modules will be installed during meson install. This should also be changed, then, as we always want to install them to '/lib/modules/<version>' (not necessarily to 'kernel_dir', as the user may change that) and only for native builds, right?

> >
> >  # 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
> >