[RFC,v4] 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     | 80 ++++++++++++++++++++++++++++++------
 2 files changed, 71 insertions(+), 17 deletions(-)
  

On Fri, Feb 05, 2021 at 04:04:32PM +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>
> ---

Thanks, this all looks ok to me now, bar one very minor nit below. Doing a
native build on my system with the running kernel also works fine. 

However, the bigger question is one of compatibility for this change. The
current documentation for the kernel_dir option is:
  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.')

Obviously the description now needs an update to reflect the new use, but
I'm not sure if changing the behaviour counts as an "ABI" change or not,
and whether it needs to wait for a new LTS release. Any scripts that were
compiling using e.g. kernel_dir='/lib/modules/<version>' need to be changed
to use kernel_dir='/lib/modules/<version>/build' instead.

/Bruce

>  kernel/linux/kni/meson.build |  8 ++--
>  kernel/linux/meson.build     | 80 ++++++++++++++++++++++++++++++------
>  2 files changed, 71 insertions(+), 17 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..7acb52944f 100644
> --- a/kernel/linux/meson.build
> +++ b/kernel/linux/meson.build
> @@ -3,25 +3,79 @@
>  
>  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')
> +install = not meson.is_cross_build()
> +cross_args = []
> +kernel_install_dir = ''

Minor nit, I'd have kernel_install_dir immediately after kernel_build_dir
in the list above.

>  
> -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
> +		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)
> -- 
> 2.20.1
>
  

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, February 5, 2021 4:27 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; david.marchand@redhat.com; bluca@debian.org
> Subject: Re: [RFC PATCH v4] build: kni cross-compilation support
> 
> On Fri, Feb 05, 2021 at 04:04:32PM +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>
> > ---
> 
> Thanks, this all looks ok to me now, bar one very minor nit below. Doing a native
> build on my system with the running kernel also works fine.
> 
> However, the bigger question is one of compatibility for this change. The current
> documentation for the kernel_dir option is:
>   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.')
> 
> Obviously the description now needs an update to reflect the new use

I'll change the description. The current patch version is always installing the modules into '/lib/modules/' + kernel_version + '/extra/dpdk', though. I don't think we want to change the behavior this way, so I'll make the changes to preserve to original behavior ('/lib/modules/' + kernel_version + '/extra/dpdk' when kernel_dir is not supplied, kernel_dir + '/extra/dpdk' when it is).

> , but I'm
> not sure if changing the behaviour counts as an "ABI" change or not, and
> whether it needs to wait for a new LTS release. Any scripts that were compiling
> using e.g. kernel_dir='/lib/modules/<version>' need to be changed to use
> kernel_dir='/lib/modules/<version>/build' instead.
> 

I'm not sure what to do with this. Should I make it backwards compatible by checking the build dir as well (i.e. trying make kernelversion in both $kernel_dir and $kernel_dir/build)?

> /Bruce
> 
> >  kernel/linux/kni/meson.build |  8 ++--
> >  kernel/linux/meson.build     | 80 ++++++++++++++++++++++++++++++------
> >  2 files changed, 71 insertions(+), 17 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..7acb52944f 100644
> > --- a/kernel/linux/meson.build
> > +++ b/kernel/linux/meson.build
> > @@ -3,25 +3,79 @@
> >
> >  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') install = not
> > +meson.is_cross_build() cross_args = [] kernel_install_dir = ''
> 
> Minor nit, I'd have kernel_install_dir immediately after kernel_build_dir in the list
> above.
> 

Sure.
  

On Mon, Feb 08, 2021 at 10:17:56AM +0000, Juraj Linkeš wrote:
> 
> 
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Friday, February 5, 2021 4:27 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; david.marchand@redhat.com; bluca@debian.org
> > Subject: Re: [RFC PATCH v4] build: kni cross-compilation support
> > 
> > On Fri, Feb 05, 2021 at 04:04:32PM +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>
> > > ---
> > 
> > Thanks, this all looks ok to me now, bar one very minor nit below. Doing a native
> > build on my system with the running kernel also works fine.
> > 
> > However, the bigger question is one of compatibility for this change. The current
> > documentation for the kernel_dir option is:
> >   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.')
> > 
> > Obviously the description now needs an update to reflect the new use
> 
> I'll change the description. The current patch version is always installing the modules into '/lib/modules/' + kernel_version + '/extra/dpdk', though. I don't think we want to change the behavior this way, so I'll make the changes to preserve to original behavior ('/lib/modules/' + kernel_version + '/extra/dpdk' when kernel_dir is not supplied, kernel_dir + '/extra/dpdk' when it is).
> 

In the absense of an explicit kernel_install_dir, I actually think the new
way is better. However, I'd be interested in other opinions on this.


> > , but I'm
> > not sure if changing the behaviour counts as an "ABI" change or not, and
> > whether it needs to wait for a new LTS release. Any scripts that were compiling
> > using e.g. kernel_dir='/lib/modules/<version>' need to be changed to use
> > kernel_dir='/lib/modules/<version>/build' instead.
> > 
> 
> I'm not sure what to do with this. Should I make it backwards compatible by checking the build dir as well (i.e. trying make kernelversion in both $kernel_dir and $kernel_dir/build)?
> 
That's an interesting proposal. Might be worth doing to check both.
  

08/02/2021 11:26, Bruce Richardson:
> On Mon, Feb 08, 2021 at 10:17:56AM +0000, Juraj Linkeš wrote:
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > > On Fri, Feb 05, 2021 at 04:04:32PM +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>
> > > > ---
> > > 
> > > Thanks, this all looks ok to me now, bar one very minor nit below. Doing a native
> > > build on my system with the running kernel also works fine.
> > > 
> > > However, the bigger question is one of compatibility for this change. The current
> > > documentation for the kernel_dir option is:
> > >   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.')
> > > 
> > > Obviously the description now needs an update to reflect the new use
> > 
> > I'll change the description. The current patch version is always installing the modules into '/lib/modules/' + kernel_version + '/extra/dpdk', though. I don't think we want to change the behavior this way, so I'll make the changes to preserve to original behavior ('/lib/modules/' + kernel_version + '/extra/dpdk' when kernel_dir is not supplied, kernel_dir + '/extra/dpdk' when it is).
> > 
> 
> In the absense of an explicit kernel_install_dir, I actually think the new
> way is better. However, I'd be interested in other opinions on this.

I'm not following. What do you call the "new way"?
  

On Mon, Feb 08, 2021 at 11:56:21AM +0100, Thomas Monjalon wrote:
> 08/02/2021 11:26, Bruce Richardson:
> > On Mon, Feb 08, 2021 at 10:17:56AM +0000, Juraj Linkeš wrote:
> > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > On Fri, Feb 05, 2021 at 04:04:32PM +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>
> > > > > ---
> > > > 
> > > > Thanks, this all looks ok to me now, bar one very minor nit below. Doing a native
> > > > build on my system with the running kernel also works fine.
> > > > 
> > > > However, the bigger question is one of compatibility for this change. The current
> > > > documentation for the kernel_dir option is:
> > > >   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.')
> > > > 
> > > > Obviously the description now needs an update to reflect the new use
> > > 
> > > I'll change the description. The current patch version is always installing the modules into '/lib/modules/' + kernel_version + '/extra/dpdk', though. I don't think we want to change the behavior this way, so I'll make the changes to preserve to original behavior ('/lib/modules/' + kernel_version + '/extra/dpdk' when kernel_dir is not supplied, kernel_dir + '/extra/dpdk' when it is).
> > > 
> > 
> > In the absense of an explicit kernel_install_dir, I actually think the new
> > way is better. However, I'd be interested in other opinions on this.
> 
> I'm not following. What do you call the "new way"?
>

Setting the install path to /lib/modules/<version> for native builds ignoring
kernel_dir value.
  

08/02/2021 12:05, Bruce Richardson:
> On Mon, Feb 08, 2021 at 11:56:21AM +0100, Thomas Monjalon wrote:
> > 08/02/2021 11:26, Bruce Richardson:
> > > On Mon, Feb 08, 2021 at 10:17:56AM +0000, Juraj Linkeš wrote:
> > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > > On Fri, Feb 05, 2021 at 04:04:32PM +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>
> > > > > > ---
> > > > > 
> > > > > Thanks, this all looks ok to me now, bar one very minor nit below. Doing a native
> > > > > build on my system with the running kernel also works fine.
> > > > > 
> > > > > However, the bigger question is one of compatibility for this change. The current
> > > > > documentation for the kernel_dir option is:
> > > > >   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.')
> > > > > 
> > > > > Obviously the description now needs an update to reflect the new use
> > > > 
> > > > I'll change the description. The current patch version is always installing the modules into '/lib/modules/' + kernel_version + '/extra/dpdk', though. I don't think we want to change the behavior this way, so I'll make the changes to preserve to original behavior ('/lib/modules/' + kernel_version + '/extra/dpdk' when kernel_dir is not supplied, kernel_dir + '/extra/dpdk' when it is).
> > > > 
> > > 
> > > In the absense of an explicit kernel_install_dir, I actually think the new
> > > way is better. However, I'd be interested in other opinions on this.
> > 
> > I'm not following. What do you call the "new way"?
> 
> Setting the install path to /lib/modules/<version> for native builds ignoring
> kernel_dir value.

What is the advantage of ignoring an user parameter?
  

On Mon, Feb 08, 2021 at 12:21:17PM +0100, Thomas Monjalon wrote:
> 08/02/2021 12:05, Bruce Richardson:
> > On Mon, Feb 08, 2021 at 11:56:21AM +0100, Thomas Monjalon wrote:
> > > 08/02/2021 11:26, Bruce Richardson:
> > > > On Mon, Feb 08, 2021 at 10:17:56AM +0000, Juraj Linkeš wrote:
> > > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > > > On Fri, Feb 05, 2021 at 04:04:32PM +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>
> > > > > > > ---
> > > > > > 
> > > > > > Thanks, this all looks ok to me now, bar one very minor nit below. Doing a native
> > > > > > build on my system with the running kernel also works fine.
> > > > > > 
> > > > > > However, the bigger question is one of compatibility for this change. The current
> > > > > > documentation for the kernel_dir option is:
> > > > > >   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.')
> > > > > > 
> > > > > > Obviously the description now needs an update to reflect the new use
> > > > > 
> > > > > I'll change the description. The current patch version is always installing the modules into '/lib/modules/' + kernel_version + '/extra/dpdk', though. I don't think we want to change the behavior this way, so I'll make the changes to preserve to original behavior ('/lib/modules/' + kernel_version + '/extra/dpdk' when kernel_dir is not supplied, kernel_dir + '/extra/dpdk' when it is).
> > > > > 
> > > > 
> > > > In the absense of an explicit kernel_install_dir, I actually think the new
> > > > way is better. However, I'd be interested in other opinions on this.
> > > 
> > > I'm not following. What do you call the "new way"?
> > 
> > Setting the install path to /lib/modules/<version> for native builds ignoring
> > kernel_dir value.
> 
> What is the advantage of ignoring an user parameter?
> 
Because the kernel_dir parameter is primarily specifying the build
directory for kmods, not the install dir. If kernel_dir is given as
"/home/user/kernel/src/linux", for example, the it's generally not wanted
to install the modules to a subdirectory of that path. If, on the other
hand, the kernel_dir value is given as "/lib/modules/<version>" then we can
use that as the basis for an install, but we also hit the challenge as to
whether the kernel_dir value should be with or without the "/build" suffix
for the /lib/modules directory.

/Bruce
  

08/02/2021 12:45, Bruce Richardson:
> On Mon, Feb 08, 2021 at 12:21:17PM +0100, Thomas Monjalon wrote:
> > 08/02/2021 12:05, Bruce Richardson:
> > > On Mon, Feb 08, 2021 at 11:56:21AM +0100, Thomas Monjalon wrote:
> > > > 08/02/2021 11:26, Bruce Richardson:
> > > > > On Mon, Feb 08, 2021 at 10:17:56AM +0000, Juraj Linkeš wrote:
> > > > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > > > > On Fri, Feb 05, 2021 at 04:04:32PM +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>
> > > > > > > > ---
> > > > > > > 
> > > > > > > Thanks, this all looks ok to me now, bar one very minor nit below. Doing a native
> > > > > > > build on my system with the running kernel also works fine.
> > > > > > > 
> > > > > > > However, the bigger question is one of compatibility for this change. The current
> > > > > > > documentation for the kernel_dir option is:
> > > > > > >   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.')
> > > > > > > 
> > > > > > > Obviously the description now needs an update to reflect the new use
> > > > > > 
> > > > > > I'll change the description. The current patch version is always installing the modules into '/lib/modules/' + kernel_version + '/extra/dpdk', though. I don't think we want to change the behavior this way, so I'll make the changes to preserve to original behavior ('/lib/modules/' + kernel_version + '/extra/dpdk' when kernel_dir is not supplied, kernel_dir + '/extra/dpdk' when it is).
> > > > > > 
> > > > > 
> > > > > In the absense of an explicit kernel_install_dir, I actually think the new
> > > > > way is better. However, I'd be interested in other opinions on this.
> > > > 
> > > > I'm not following. What do you call the "new way"?
> > > 
> > > Setting the install path to /lib/modules/<version> for native builds ignoring
> > > kernel_dir value.
> > 
> > What is the advantage of ignoring an user parameter?
> > 
> Because the kernel_dir parameter is primarily specifying the build
> directory for kmods, not the install dir. If kernel_dir is given as
> "/home/user/kernel/src/linux", for example, the it's generally not wanted
> to install the modules to a subdirectory of that path. If, on the other
> hand, the kernel_dir value is given as "/lib/modules/<version>" then we can
> use that as the basis for an install, but we also hit the challenge as to
> whether the kernel_dir value should be with or without the "/build" suffix
> for the /lib/modules directory.

In the case of native build, isn't the src directory standardized?
In my case, it is /lib/modules/version/kernel/
so I would assume that giving a kernel_dir means both src and install
directories are requested to be somewhere else.

If there is no standard kernel source path,
then I understand kernel_dir may be used without assuming
the install directory would take kernel_dir into account.