[v5] build: kni cross-compilation support

  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>
---
 kernel/linux/kni/meson.build |  8 +--
 kernel/linux/meson.build     | 94 +++++++++++++++++++++++++++++++-----
 meson_options.txt            |  2 +-
 3 files changed, 86 insertions(+), 18 deletions(-)
  

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š <juraj.linkes@pantheon.tech>

Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>

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
>
  

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Tuesday, February 9, 2021 12:51 PM
> 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: [PATCH v5] build: kni cross-compilation support
> 
> 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š <juraj.linkes@pantheon.tech>
> 
> Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> 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.
> 

Ok, I'll remove the lines.

> > +		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".
> 

Having it in docs would be better, but where exactly? I tried looking, but didn't find a suitable place.

> >  option('machine', type: 'string', value: 'native',
> >  	description: 'set the target machine type')  option('max_ethports',
> > type: 'integer', value: 32,
> > --
> > 2.20.1
> >
  

On Tue, Feb 09, 2021 at 12:07:13PM +0000, Juraj Linkeš wrote:
> 
> 
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Tuesday, February 9, 2021 12:51 PM
> > 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: [PATCH v5] build: kni cross-compilation support
> > 
<snip>
> > > +	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".
> > 
> 
> Having it in docs would be better, but where exactly? I tried looking, but didn't find a suitable place.
> 

It probably should go in the GSG doc, but I agree that there is nowhere
obvious for it. I think we could do with the addition of some descriptions
of the various options in the GSG, but that is out of the scope of this
patch.