DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [RFC PATCH v1] build: kni gcc cross-compilation support
@ 2021-01-29 10:29 Juraj Linkeš
  2021-01-29 11:43 ` Bruce Richardson
  2021-02-04  9:51 ` [dpdk-dev] [RFC PATCH v2] build: kni " Juraj Linkeš
  0 siblings, 2 replies; 37+ messages in thread
From: Juraj Linkeš @ 2021-01-29 10:29 UTC (permalink / raw)
  To: bruce.richardson, thomas, Ruifeng.Wang, jerinjacobk,
	hemant.agrawal, ferruh.yigit, aboyer
  Cc: dev, Juraj Linkeš

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


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v1] build: kni gcc cross-compilation support
  2021-01-29 10:29 [dpdk-dev] [RFC PATCH v1] build: kni gcc cross-compilation support Juraj Linkeš
@ 2021-01-29 11:43 ` Bruce Richardson
  2021-01-29 12:33   ` Juraj Linkeš
  2021-02-04  9:51 ` [dpdk-dev] [RFC PATCH v2] build: kni " Juraj Linkeš
  1 sibling, 1 reply; 37+ messages in thread
From: Bruce Richardson @ 2021-01-29 11:43 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: thomas, Ruifeng.Wang, jerinjacobk, hemant.agrawal, ferruh.yigit,
	aboyer, dev

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? 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$//"')

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

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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v1] build: kni gcc cross-compilation support
  2021-01-29 11:43 ` Bruce Richardson
@ 2021-01-29 12:33   ` Juraj Linkeš
  2021-01-29 13:51     ` Bruce Richardson
  0 siblings, 1 reply; 37+ messages in thread
From: Juraj Linkeš @ 2021-01-29 12:33 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: thomas, Ruifeng.Wang, jerinjacobk, hemant.agrawal, ferruh.yigit,
	aboyer, dev



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


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v1] build: kni gcc cross-compilation support
  2021-01-29 12:33   ` Juraj Linkeš
@ 2021-01-29 13:51     ` Bruce Richardson
  2021-01-29 14:36       ` Juraj Linkeš
  0 siblings, 1 reply; 37+ messages in thread
From: Bruce Richardson @ 2021-01-29 13:51 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: thomas, Ruifeng.Wang, jerinjacobk, hemant.agrawal, ferruh.yigit,
	aboyer, dev

On Fri, Jan 29, 2021 at 12:33:06PM +0000, Juraj Linkeš wrote:
> 
> 
> > -----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?
> 
Well, we definitely want it for native builds, but I'd imagine it would be
useful for cross-builds too, no? Can we find some way of getting it working
for both cases. For native builds we want:

* build kernel-dir = /lib/modules/<ver>/build
* install dir = /lib/modules/<ver>/extra/dpdk

What are the expected equivalent paths for cross building?

/Bruce

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v1] build: kni gcc cross-compilation support
  2021-01-29 13:51     ` Bruce Richardson
@ 2021-01-29 14:36       ` Juraj Linkeš
  2021-01-29 14:42         ` Bruce Richardson
  0 siblings, 1 reply; 37+ messages in thread
From: Juraj Linkeš @ 2021-01-29 14:36 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: thomas, Ruifeng.Wang, jerinjacobk, hemant.agrawal, ferruh.yigit,
	aboyer, dev



> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, January 29, 2021 2:51 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 12:33:06PM +0000, Juraj Linkeš wrote:
> >
> >
> > > -----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?
> >
> Well, we definitely want it for native builds, but I'd imagine it would be useful for
> cross-builds too, no?

I guess it would be useful for setups with shared storage. Did you have something else in mind?

> Can we find some way of getting it working for both cases.
> For native builds we want:
> 
> * build kernel-dir = /lib/modules/<ver>/build
> * install dir = /lib/modules/<ver>/extra/dpdk
> 
> What are the expected equivalent paths for cross building?
> 

The ubuntu1804 packages are installing aarch64 cross files to /usr/aarch64-linux-gnu, so we could install it to /usr/aarch64-linux-gnu/lib/modules/<ver>/extra/dpdk, or /usr/<cross_triple>/lib/modules/<ver>/extra/dpdk in general. I think we can get <ver> from 'make kernelversion', so that would work, although I'm not sure this is the right place.

A note: all this works just for gcc. Should I also add support for clang?

> /Bruce


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v1] build: kni gcc cross-compilation support
  2021-01-29 14:36       ` Juraj Linkeš
@ 2021-01-29 14:42         ` Bruce Richardson
  2021-01-29 14:47           ` Juraj Linkeš
  0 siblings, 1 reply; 37+ messages in thread
From: Bruce Richardson @ 2021-01-29 14:42 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: thomas, Ruifeng.Wang, jerinjacobk, hemant.agrawal, ferruh.yigit,
	aboyer, dev

On Fri, Jan 29, 2021 at 02:36:58PM +0000, Juraj Linkeš wrote:
> 
> 
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Friday, January 29, 2021 2:51 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 12:33:06PM +0000, Juraj Linkeš wrote:
> > >
> > >
> > > > -----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?
> > >
> > Well, we definitely want it for native builds, but I'd imagine it would be useful for
> > cross-builds too, no?
> 
> I guess it would be useful for setups with shared storage. Did you have something else in mind?
> 
> > Can we find some way of getting it working for both cases.
> > For native builds we want:
> > 
> > * build kernel-dir = /lib/modules/<ver>/build
> > * install dir = /lib/modules/<ver>/extra/dpdk
> > 
> > What are the expected equivalent paths for cross building?
> > 
> 
> The ubuntu1804 packages are installing aarch64 cross files to /usr/aarch64-linux-gnu, so we could install it to /usr/aarch64-linux-gnu/lib/modules/<ver>/extra/dpdk, or /usr/<cross_triple>/lib/modules/<ver>/extra/dpdk in general. I think we can get <ver> from 'make kernelversion', so that would work, although I'm not sure this is the right place.
> 
So what do you specify as the "kernel_dir" for the cross compile?

> A note: all this works just for gcc. Should I also add support for clang?
> 
If possible, that would be great.

/Bruce

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v1] build: kni gcc cross-compilation support
  2021-01-29 14:42         ` Bruce Richardson
@ 2021-01-29 14:47           ` Juraj Linkeš
  2021-01-29 15:01             ` Bruce Richardson
  0 siblings, 1 reply; 37+ messages in thread
From: Juraj Linkeš @ 2021-01-29 14:47 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: thomas, Ruifeng.Wang, jerinjacobk, hemant.agrawal, ferruh.yigit,
	aboyer, dev



> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, January 29, 2021 3:42 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 02:36:58PM +0000, Juraj Linkeš wrote:
> >
> >
> > > -----Original Message-----
> > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > Sent: Friday, January 29, 2021 2:51 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 12:33:06PM +0000, Juraj Linkeš wrote:
> > > >
> > > >
> > > > > -----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?
> > > >
> > > Well, we definitely want it for native builds, but I'd imagine it
> > > would be useful for cross-builds too, no?
> >
> > I guess it would be useful for setups with shared storage. Did you have
> something else in mind?
> >
> > > Can we find some way of getting it working for both cases.
> > > For native builds we want:
> > >
> > > * build kernel-dir = /lib/modules/<ver>/build
> > > * install dir = /lib/modules/<ver>/extra/dpdk
> > >
> > > What are the expected equivalent paths for cross building?
> > >
> >
> > The ubuntu1804 packages are installing aarch64 cross files to /usr/aarch64-
> linux-gnu, so we could install it to /usr/aarch64-linux-
> gnu/lib/modules/<ver>/extra/dpdk, or
> /usr/<cross_triple>/lib/modules/<ver>/extra/dpdk in general. I think we can get
> <ver> from 'make kernelversion', so that would work, although I'm not sure this
> is the right place.
> >
> So what do you specify as the "kernel_dir" for the cross compile?
> 

The place where I cloned (and cross-compiled) linux sources: $HOME/linux.

> > A note: all this works just for gcc. Should I also add support for clang?
> >
> If possible, that would be great.
> 

Ok, I'll look into it.

> /Bruce


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v1] build: kni gcc cross-compilation support
  2021-01-29 14:47           ` Juraj Linkeš
@ 2021-01-29 15:01             ` Bruce Richardson
  2021-01-29 15:17               ` Juraj Linkeš
  0 siblings, 1 reply; 37+ messages in thread
From: Bruce Richardson @ 2021-01-29 15:01 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: thomas, Ruifeng.Wang, jerinjacobk, hemant.agrawal, ferruh.yigit,
	aboyer, dev

On Fri, Jan 29, 2021 at 02:47:57PM +0000, Juraj Linkeš wrote:
> 
> 
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Friday, January 29, 2021 3:42 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
> > 
<snip>
> > > > Can we find some way of getting it working for both cases.
> > > > For native builds we want:
> > > >
> > > > * build kernel-dir = /lib/modules/<ver>/build
> > > > * install dir = /lib/modules/<ver>/extra/dpdk
> > > >
> > > > What are the expected equivalent paths for cross building?
> > > >
> > >
> > > The ubuntu1804 packages are installing aarch64 cross files to /usr/aarch64-
> > linux-gnu, so we could install it to /usr/aarch64-linux-
> > gnu/lib/modules/<ver>/extra/dpdk, or
> > /usr/<cross_triple>/lib/modules/<ver>/extra/dpdk in general. I think we can get
> > <ver> from 'make kernelversion', so that would work, although I'm not sure this
> > is the right place.
> > >
> > So what do you specify as the "kernel_dir" for the cross compile?
> > 
> 
> The place where I cloned (and cross-compiled) linux sources: $HOME/linux.
> 
So I think the key problem is that for cross-compilation you need two
completely independent paths, while for native builds the two can be
linked. Is that correct?

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v1] build: kni gcc cross-compilation support
  2021-01-29 15:01             ` Bruce Richardson
@ 2021-01-29 15:17               ` Juraj Linkeš
  2021-01-29 15:39                 ` Bruce Richardson
  0 siblings, 1 reply; 37+ messages in thread
From: Juraj Linkeš @ 2021-01-29 15:17 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: thomas, Ruifeng.Wang, jerinjacobk, hemant.agrawal, ferruh.yigit,
	aboyer, dev



> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, January 29, 2021 4:01 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 02:47:57PM +0000, Juraj Linkeš wrote:
> >
> >
> > > -----Original Message-----
> > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > Sent: Friday, January 29, 2021 3:42 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
> > >
> <snip>
> > > > > Can we find some way of getting it working for both cases.
> > > > > For native builds we want:
> > > > >
> > > > > * build kernel-dir = /lib/modules/<ver>/build
> > > > > * install dir = /lib/modules/<ver>/extra/dpdk
> > > > >
> > > > > What are the expected equivalent paths for cross building?
> > > > >
> > > >
> > > > The ubuntu1804 packages are installing aarch64 cross files to
> > > > /usr/aarch64-
> > > linux-gnu, so we could install it to /usr/aarch64-linux-
> > > gnu/lib/modules/<ver>/extra/dpdk, or
> > > /usr/<cross_triple>/lib/modules/<ver>/extra/dpdk in general. I think
> > > we can get <ver> from 'make kernelversion', so that would work,
> > > although I'm not sure this is the right place.
> > > >
> > > So what do you specify as the "kernel_dir" for the cross compile?
> > >
> >
> > The place where I cloned (and cross-compiled) linux sources: $HOME/linux.
> >
> So I think the key problem is that for cross-compilation you need two completely
> independent paths, while for native builds the two can be linked. Is that correct?

Assuming that we want to install cross modules, then yes, although it's incomplete. There's also the scenario when a user would use different kernel_dir than the default for native builds. I don't know why anyone would do that, but it is a theoretical possibility :-)


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v1] build: kni gcc cross-compilation support
  2021-01-29 15:17               ` Juraj Linkeš
@ 2021-01-29 15:39                 ` Bruce Richardson
  2021-02-01  7:48                   ` Juraj Linkeš
  0 siblings, 1 reply; 37+ messages in thread
From: Bruce Richardson @ 2021-01-29 15:39 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: thomas, Ruifeng.Wang, jerinjacobk, hemant.agrawal, ferruh.yigit,
	aboyer, dev

On Fri, Jan 29, 2021 at 03:17:02PM +0000, Juraj Linkeš wrote:
> 
> 
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Friday, January 29, 2021 4:01 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 02:47:57PM +0000, Juraj Linkeš wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > Sent: Friday, January 29, 2021 3:42 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
> > > >
> > <snip>
> > > > > > Can we find some way of getting it working for both cases.
> > > > > > For native builds we want:
> > > > > >
> > > > > > * build kernel-dir = /lib/modules/<ver>/build
> > > > > > * install dir = /lib/modules/<ver>/extra/dpdk
> > > > > >
> > > > > > What are the expected equivalent paths for cross building?
> > > > > >
> > > > >
> > > > > The ubuntu1804 packages are installing aarch64 cross files to
> > > > > /usr/aarch64-
> > > > linux-gnu, so we could install it to /usr/aarch64-linux-
> > > > gnu/lib/modules/<ver>/extra/dpdk, or
> > > > /usr/<cross_triple>/lib/modules/<ver>/extra/dpdk in general. I think
> > > > we can get <ver> from 'make kernelversion', so that would work,
> > > > although I'm not sure this is the right place.
> > > > >
> > > > So what do you specify as the "kernel_dir" for the cross compile?
> > > >
> > >
> > > The place where I cloned (and cross-compiled) linux sources: $HOME/linux.
> > >
> > So I think the key problem is that for cross-compilation you need two completely
> > independent paths, while for native builds the two can be linked. Is that correct?
> 
> Assuming that we want to install cross modules, then yes, although it's incomplete. There's also the scenario when a user would use different kernel_dir than the default for native builds. I don't know why anyone would do that, but it is a theoretical possibility :-)

In the cross-compile install case, is the prefix /usr/aarch64-linux-gnu/
coming from a DESTDIR environment variable? If it is, then we could just
use the kernel_dir option for the build path, and always install to
/lib/modules/<ver>/... allowing any cross-compiler relocation to take care
of the rest. 

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v1] build: kni gcc cross-compilation support
  2021-01-29 15:39                 ` Bruce Richardson
@ 2021-02-01  7:48                   ` Juraj Linkeš
  0 siblings, 0 replies; 37+ messages in thread
From: Juraj Linkeš @ 2021-02-01  7:48 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: thomas, Ruifeng.Wang, jerinjacobk, hemant.agrawal, ferruh.yigit,
	aboyer, dev



> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, January 29, 2021 4:40 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 03:17:02PM +0000, Juraj Linkeš wrote:
> >
> >
> > > -----Original Message-----
> > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > Sent: Friday, January 29, 2021 4:01 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 02:47:57PM +0000, Juraj Linkeš wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > > Sent: Friday, January 29, 2021 3:42 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
> > > > >
> > > <snip>
> > > > > > > Can we find some way of getting it working for both cases.
> > > > > > > For native builds we want:
> > > > > > >
> > > > > > > * build kernel-dir = /lib/modules/<ver>/build
> > > > > > > * install dir = /lib/modules/<ver>/extra/dpdk
> > > > > > >
> > > > > > > What are the expected equivalent paths for cross building?
> > > > > > >
> > > > > >
> > > > > > The ubuntu1804 packages are installing aarch64 cross files to
> > > > > > /usr/aarch64-
> > > > > linux-gnu, so we could install it to /usr/aarch64-linux-
> > > > > gnu/lib/modules/<ver>/extra/dpdk, or
> > > > > /usr/<cross_triple>/lib/modules/<ver>/extra/dpdk in general. I
> > > > > think we can get <ver> from 'make kernelversion', so that would
> > > > > work, although I'm not sure this is the right place.
> > > > > >
> > > > > So what do you specify as the "kernel_dir" for the cross compile?
> > > > >
> > > >
> > > > The place where I cloned (and cross-compiled) linux sources: $HOME/linux.
> > > >
> > > So I think the key problem is that for cross-compilation you need
> > > two completely independent paths, while for native builds the two can be
> linked. Is that correct?
> >
> > Assuming that we want to install cross modules, then yes, although
> > it's incomplete. There's also the scenario when a user would use
> > different kernel_dir than the default for native builds. I don't know
> > why anyone would do that, but it is a theoretical possibility :-)
> 
> In the cross-compile install case, is the prefix /usr/aarch64-linux-gnu/ coming
> from a DESTDIR environment variable? If it is, then we could just use the
> kernel_dir option for the build path, and always install to /lib/modules/<ver>/...
> allowing any cross-compiler relocation to take care of the rest.

It's not coming from anywhere. What I meant is that an ubuntu package such as 'libatomic1-arm64-cross' installs its headers/libs under /usr/aarch64-linux-gnu/.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [dpdk-dev] [RFC PATCH v2] build: kni cross-compilation support
  2021-01-29 10:29 [dpdk-dev] [RFC PATCH v1] build: kni gcc cross-compilation support Juraj Linkeš
  2021-01-29 11:43 ` Bruce Richardson
@ 2021-02-04  9:51 ` Juraj Linkeš
  2021-02-04 17:33   ` Bruce Richardson
  2021-02-05 14:46   ` [dpdk-dev] [RFC PATCH v3] " Juraj Linkeš
  1 sibling, 2 replies; 37+ messages in thread
From: Juraj Linkeš @ 2021-02-04  9:51 UTC (permalink / raw)
  To: bruce.richardson, thomas, Ruifeng.Wang, Honnappa.Nagarahalli,
	jerinjacobk, hemant.agrawal, ferruh.yigit, aboyer
  Cc: dev, Juraj Linkeš

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     | 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'))
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()
+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
+		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
 
 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


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v2] build: kni cross-compilation support
  2021-02-04  9:51 ` [dpdk-dev] [RFC PATCH v2] build: kni " Juraj Linkeš
@ 2021-02-04 17:33   ` Bruce Richardson
  2021-02-05  9:26     ` Juraj Linkeš
  2021-02-05 14:46   ` [dpdk-dev] [RFC PATCH v3] " Juraj Linkeš
  1 sibling, 1 reply; 37+ messages in thread
From: Bruce Richardson @ 2021-02-04 17:33 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: thomas, Ruifeng.Wang, Honnappa.Nagarahalli, jerinjacobk,
	hemant.agrawal, ferruh.yigit, aboyer, dev

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
> 

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v2] build: kni cross-compilation support
  2021-02-04 17:33   ` Bruce Richardson
@ 2021-02-05  9:26     ` Juraj Linkeš
  2021-02-05  9:38       ` Bruce Richardson
  2021-02-05  9:42       ` Bruce Richardson
  0 siblings, 2 replies; 37+ messages in thread
From: Juraj Linkeš @ 2021-02-05  9:26 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: thomas, Ruifeng.Wang, Honnappa.Nagarahalli, jerinjacobk,
	hemant.agrawal, ferruh.yigit, aboyer, dev



> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Thursday, February 4, 2021 6:34 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: [RFC PATCH v2] build: kni cross-compilation support
> 
> 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.
> 

Thanks, these are very helpful.

> /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.
> 

Makes sense, I'll do that.

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

In meson vernicular the host machine is where the binaries will be running, i.e. what we're building for, so this may be a bit confusing - we could name it build_machine_kernel_version.
In any case, I'll move to the the native build if branch. But then maybe we don't need to rename it?

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

Ok, I'll make the default true and set it to false in the cross compilation case.

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

This configuration would be the same for all kernel modules (right?), so I'm not sure how the number of kernel modules is relevant here.
If we split it, what would the dir structure look like? Something like this?
kernel/linux/
├── aarch64
├── native
├── kni
├── <other_mods>

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

Wouldn't we need to duplicate the code that does make kernelversion and subdirs the actaul modules?

> Also, throughout the block, anywhere you have an "error()" call you can use
> "endif" instead of "else" and save more indentation space.
> 

Good point, I'll make the change.

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


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v2] build: kni cross-compilation support
  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
  1 sibling, 1 reply; 37+ messages in thread
From: Bruce Richardson @ 2021-02-05  9:38 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: thomas, Ruifeng.Wang, Honnappa.Nagarahalli, jerinjacobk,
	hemant.agrawal, ferruh.yigit, aboyer, dev

On Fri, Feb 05, 2021 at 09:26:05AM +0000, Juraj Linkeš wrote:
> 
> 
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Thursday, February 4, 2021 6:34 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: [RFC PATCH v2] build: kni cross-compilation support
> > 
> > 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.
> > 
> 
> Thanks, these are very helpful.
> 
<snip>
> > >
> > > +kernel_version = run_command('uname', '-r').stdout().strip()
> > 
> > Rename to "host_kernel_version" and probably should be only queried in the
> > native build case.
> > 
> 
> In meson vernicular the host machine is where the binaries will be running, i.e. what we're building for, so this may be a bit confusing - we could name it build_machine_kernel_version.
> In any case, I'll move to the the native build if branch. But then maybe we don't need to rename it?
> 
Agreed.

> > > +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()
<snip>
> > >  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.
> 
> This configuration would be the same for all kernel modules (right?), so I'm not sure how the number of kernel modules is relevant here.
> If we split it, what would the dir structure look like? Something like this?
> kernel/linux/
> ├── aarch64
> ├── native
> ├── kni
> ├── <other_mods>
> 

Yep, that would be the structure - though perhaps with "cross" rather than
"aarch64" being the alternative to "native".  The reason I felt that the
number of kmods was relevant is that it would be really weird to have 2
subdirectories for infrastructure for a single directory containing one
module - a 200% percent overhead ratio. :-) Therefore, I thinking keeping
it all in one file is best, but we'll see after the next revision how it
looks.

> > 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.
> > 
> 
> Wouldn't we need to duplicate the code that does make kernelversion and subdirs the actaul modules?
> 

Definitely yes for the subdir call, but that is only one line now. The
"make kernelversion" call - is that needed in the cross-compile case since
one explicitly has to pass in the kernel directories? My original thinking
was to use it to check for the sources more in the native case where we are
picking up paths automatically. Again, it's only a line or two duplicated.
Alternatively, that check could be moved to the "kni" directory meson.build
file, which would make it common.


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v2] build: kni cross-compilation support
  2021-02-05  9:26     ` Juraj Linkeš
  2021-02-05  9:38       ` Bruce Richardson
@ 2021-02-05  9:42       ` Bruce Richardson
  1 sibling, 0 replies; 37+ messages in thread
From: Bruce Richardson @ 2021-02-05  9:42 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: thomas, Ruifeng.Wang, Honnappa.Nagarahalli, jerinjacobk,
	hemant.agrawal, ferruh.yigit, aboyer, dev

On Fri, Feb 05, 2021 at 09:26:05AM +0000, Juraj Linkeš wrote:
> 
> 
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Thursday, February 4, 2021 6:34 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: [RFC PATCH v2] build: kni cross-compilation support
> > 
> > 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.
> > 
> 
> Thanks, these are very helpful.
> 
<snip>
> > > +		install_dir = ''
> > > +		install = false
> > 
> > I think these should be defined and initialized further up the file, outside the
> > conditional block.
> > 
> 
> Ok, I'll make the default true and set it to false in the cross compilation case.
> 
Can you just initialize the value to !meson.is_cross_build() and leave it
at that?

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v2] build: kni cross-compilation support
  2021-02-05  9:38       ` Bruce Richardson
@ 2021-02-05  9:44         ` Thomas Monjalon
  0 siblings, 0 replies; 37+ messages in thread
From: Thomas Monjalon @ 2021-02-05  9:44 UTC (permalink / raw)
  To: Juraj Linkeš, Bruce Richardson
  Cc: Ruifeng.Wang, Honnappa.Nagarahalli, jerinjacobk, hemant.agrawal,
	ferruh.yigit, aboyer, dev

05/02/2021 10:38, Bruce Richardson:
> On Fri, Feb 05, 2021 at 09:26:05AM +0000, Juraj Linkeš wrote:
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > > 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.
> > 
> > This configuration would be the same for all kernel modules (right?), so I'm not sure how the number of kernel modules is relevant here.
> > If we split it, what would the dir structure look like? Something like this?
> > kernel/linux/
> > ├── aarch64
> > ├── native
> > ├── kni
> > ├── <other_mods>
> > 
> 
> Yep, that would be the structure - though perhaps with "cross" rather than
> "aarch64" being the alternative to "native".  The reason I felt that the
> number of kmods was relevant is that it would be really weird to have 2
> subdirectories for infrastructure for a single directory containing one
> module - a 200% percent overhead ratio. :-) Therefore, I thinking keeping
> it all in one file is best, but we'll see after the next revision how it
> looks.

Going forward, we should not have any Linux kernel module in this DPDK repository.
We must encourage upstream developments.
That's why I am against adding more directories in kernel/linux/.
KNI is still there (could move later), but please handle it in kni/ directory.




^ permalink raw reply	[flat|nested] 37+ messages in thread

* [dpdk-dev] [RFC PATCH v3] build: kni cross-compilation support
  2021-02-04  9:51 ` [dpdk-dev] [RFC PATCH v2] build: kni " Juraj Linkeš
  2021-02-04 17:33   ` Bruce Richardson
@ 2021-02-05 14:46   ` Juraj Linkeš
  2021-02-05 14:52     ` Bruce Richardson
  2021-02-05 15:04     ` [dpdk-dev] [RFC PATCH v4] " Juraj Linkeš
  1 sibling, 2 replies; 37+ messages in thread
From: Juraj Linkeš @ 2021-02-05 14:46 UTC (permalink / raw)
  To: bruce.richardson, thomas, Ruifeng.Wang, Honnappa.Nagarahalli,
	jerinjacobk, hemant.agrawal, ferruh.yigit, aboyer
  Cc: dev, Juraj Linkeš

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     | 79 ++++++++++++++++++++++++++++++------
 2 files changed, 70 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..4271483329 100644
--- a/kernel/linux/meson.build
+++ b/kernel/linux/meson.build
@@ -3,24 +3,77 @@
 
 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 = ''
 
-kernel_dir = get_option('kernel_dir')
-if kernel_dir == ''
-	# use default path for native builds
+if install
+	# 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
+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)]
 
 # DO ACTUAL MODULE BUILDING
 foreach d:subdirs
-- 
2.20.1


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v3] build: kni cross-compilation support
  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š
  1 sibling, 1 reply; 37+ messages in thread
From: Bruce Richardson @ 2021-02-05 14:52 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: thomas, Ruifeng.Wang, Honnappa.Nagarahalli, jerinjacobk,
	hemant.agrawal, ferruh.yigit, aboyer, dev

On Fri, Feb 05, 2021 at 03:46:37PM +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>
> ---
>  kernel/linux/kni/meson.build |  8 ++--
>  kernel/linux/meson.build     | 79 ++++++++++++++++++++++++++++++------
>  2 files changed, 70 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..4271483329 100644
> --- a/kernel/linux/meson.build
> +++ b/kernel/linux/meson.build
> @@ -3,24 +3,77 @@
>  
>  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 = ''
>  
> -kernel_dir = get_option('kernel_dir')
> -if kernel_dir == ''
> -	# use default path for native builds
> +if install

I'd suggest using explicit "not meson.is_cross_build" again for
readability.

> +	# 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
> +if host_machine.cpu_family() == 'aarch64'
> +	cross_arch = 'arm64'
> +else
> +	cross_arch = build_machine.cpu_family()
>  endif

Is this not meant to be "host_machine.cpu_family()"?
I also think this might be simpler as:

cross_arch = host_machine.cpu_family()
if cross_arch == '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
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v3] build: kni cross-compilation support
  2021-02-05 14:52     ` Bruce Richardson
@ 2021-02-05 15:02       ` Juraj Linkeš
  0 siblings, 0 replies; 37+ messages in thread
From: Juraj Linkeš @ 2021-02-05 15:02 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: thomas, Ruifeng.Wang, Honnappa.Nagarahalli, jerinjacobk,
	hemant.agrawal, ferruh.yigit, aboyer, dev



> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, February 5, 2021 3:53 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: [RFC PATCH v3] build: kni cross-compilation support
> 
> On Fri, Feb 05, 2021 at 03:46:37PM +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>
> > ---
> >  kernel/linux/kni/meson.build |  8 ++--
> >  kernel/linux/meson.build     | 79 ++++++++++++++++++++++++++++++------
> >  2 files changed, 70 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..4271483329 100644
> > --- a/kernel/linux/meson.build
> > +++ b/kernel/linux/meson.build
> > @@ -3,24 +3,77 @@
> >
> >  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 = ''
> >
> > -kernel_dir = get_option('kernel_dir') -if kernel_dir == ''
> > -	# use default path for native builds
> > +if install
> 
> I'd suggest using explicit "not meson.is_cross_build" again for readability.
> 

Ok.

> > +	# 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 if host_machine.cpu_family() == 'aarch64'
> > +	cross_arch = 'arm64'
> > +else
> > +	cross_arch = build_machine.cpu_family()
> >  endif
> 
> Is this not meant to be "host_machine.cpu_family()"?
> I also think this might be simpler as:
> 
> cross_arch = host_machine.cpu_family()
> if cross_arch == 'aarch64'
> 	cross_arch = arm64
> endif
> 

It should, I'll change it.

> > +cross_args += ['ARCH=@0@'.format(cross_arch),
> > +	'CROSS_COMPILE=@0@'.format(cross_prefix)]
> >
> >  # DO ACTUAL MODULE BUILDING
> >  foreach d:subdirs
> > --
> > 2.20.1
> >


^ permalink raw reply	[flat|nested] 37+ messages in thread

* [dpdk-dev] [RFC PATCH v4] build: kni cross-compilation support
  2021-02-05 14:46   ` [dpdk-dev] [RFC PATCH v3] " Juraj Linkeš
  2021-02-05 14:52     ` Bruce Richardson
@ 2021-02-05 15:04     ` Juraj Linkeš
  2021-02-05 15:27       ` Bruce Richardson
  2021-02-09  8:47       ` [dpdk-dev] [PATCH v5] " Juraj Linkeš
  1 sibling, 2 replies; 37+ messages in thread
From: Juraj Linkeš @ 2021-02-05 15:04 UTC (permalink / raw)
  To: bruce.richardson, thomas, Ruifeng.Wang, Honnappa.Nagarahalli,
	jerinjacobk, hemant.agrawal, ferruh.yigit, aboyer
  Cc: dev, Juraj Linkeš

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(-)

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 = ''
 
-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


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v4] build: kni cross-compilation support
  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-09  8:47       ` [dpdk-dev] [PATCH v5] " Juraj Linkeš
  1 sibling, 1 reply; 37+ messages in thread
From: Bruce Richardson @ 2021-02-05 15:27 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: thomas, Ruifeng.Wang, Honnappa.Nagarahalli, jerinjacobk,
	hemant.agrawal, ferruh.yigit, aboyer, dev, david.marchand, bluca

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
> 

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v4] build: kni cross-compilation support
  2021-02-05 15:27       ` Bruce Richardson
@ 2021-02-08 10:17         ` Juraj Linkeš
  2021-02-08 10:26           ` Bruce Richardson
  0 siblings, 1 reply; 37+ messages in thread
From: Juraj Linkeš @ 2021-02-08 10:17 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: thomas, Ruifeng.Wang, Honnappa.Nagarahalli, jerinjacobk,
	hemant.agrawal, ferruh.yigit, aboyer, dev, david.marchand, bluca



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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v4] build: kni cross-compilation support
  2021-02-08 10:17         ` Juraj Linkeš
@ 2021-02-08 10:26           ` Bruce Richardson
  2021-02-08 10:56             ` Thomas Monjalon
  0 siblings, 1 reply; 37+ messages in thread
From: Bruce Richardson @ 2021-02-08 10:26 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: thomas, Ruifeng.Wang, Honnappa.Nagarahalli, jerinjacobk,
	hemant.agrawal, ferruh.yigit, aboyer, dev, david.marchand, bluca

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.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v4] build: kni cross-compilation support
  2021-02-08 10:26           ` Bruce Richardson
@ 2021-02-08 10:56             ` Thomas Monjalon
  2021-02-08 11:05               ` Bruce Richardson
  0 siblings, 1 reply; 37+ messages in thread
From: Thomas Monjalon @ 2021-02-08 10:56 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Juraj Linkeš,
	Ruifeng.Wang, Honnappa.Nagarahalli, jerinjacobk, hemant.agrawal,
	ferruh.yigit, aboyer, dev, david.marchand, bluca

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"?



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v4] build: kni cross-compilation support
  2021-02-08 10:56             ` Thomas Monjalon
@ 2021-02-08 11:05               ` Bruce Richardson
  2021-02-08 11:21                 ` Thomas Monjalon
  0 siblings, 1 reply; 37+ messages in thread
From: Bruce Richardson @ 2021-02-08 11:05 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Juraj Linkeš,
	Ruifeng.Wang, Honnappa.Nagarahalli, jerinjacobk, hemant.agrawal,
	ferruh.yigit, aboyer, dev, david.marchand, bluca

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.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v4] build: kni cross-compilation support
  2021-02-08 11:05               ` Bruce Richardson
@ 2021-02-08 11:21                 ` Thomas Monjalon
  2021-02-08 11:45                   ` Bruce Richardson
  0 siblings, 1 reply; 37+ messages in thread
From: Thomas Monjalon @ 2021-02-08 11:21 UTC (permalink / raw)
  To: Bruce Richardson, Juraj Linkeš
  Cc: Ruifeng.Wang, Honnappa.Nagarahalli, jerinjacobk, hemant.agrawal,
	ferruh.yigit, aboyer, dev, david.marchand, bluca

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?



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v4] build: kni cross-compilation support
  2021-02-08 11:21                 ` Thomas Monjalon
@ 2021-02-08 11:45                   ` Bruce Richardson
  2021-02-08 17:23                     ` Thomas Monjalon
  0 siblings, 1 reply; 37+ messages in thread
From: Bruce Richardson @ 2021-02-08 11:45 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Juraj Linkeš,
	Ruifeng.Wang, Honnappa.Nagarahalli, jerinjacobk, hemant.agrawal,
	ferruh.yigit, aboyer, dev, david.marchand, bluca

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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v4] build: kni cross-compilation support
  2021-02-08 11:45                   ` Bruce Richardson
@ 2021-02-08 17:23                     ` Thomas Monjalon
  0 siblings, 0 replies; 37+ messages in thread
From: Thomas Monjalon @ 2021-02-08 17:23 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Juraj Linkeš,
	Ruifeng.Wang, Honnappa.Nagarahalli, jerinjacobk, hemant.agrawal,
	ferruh.yigit, aboyer, dev, david.marchand, bluca

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.



^ permalink raw reply	[flat|nested] 37+ messages in thread

* [dpdk-dev] [PATCH v5] build: kni cross-compilation support
  2021-02-05 15:04     ` [dpdk-dev] [RFC PATCH v4] " Juraj Linkeš
  2021-02-05 15:27       ` Bruce Richardson
@ 2021-02-09  8:47       ` Juraj Linkeš
  2021-02-09 11:50         ` Bruce Richardson
  2021-02-11 12:59         ` [dpdk-dev] [PATCH v6] " Juraj Linkeš
  1 sibling, 2 replies; 37+ messages in thread
From: Juraj Linkeš @ 2021-02-09  8:47 UTC (permalink / raw)
  To: bruce.richardson, thomas, Ruifeng.Wang, Honnappa.Nagarahalli,
	jerinjacobk, hemant.agrawal, ferruh.yigit, aboyer
  Cc: dev, Juraj Linkeš

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(-)

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
+		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.')
 option('machine', type: 'string', value: 'native',
 	description: 'set the target machine type')
 option('max_ethports', type: 'integer', value: 32,
-- 
2.20.1


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [PATCH v5] build: kni cross-compilation support
  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-11 12:59         ` [dpdk-dev] [PATCH v6] " Juraj Linkeš
  1 sibling, 1 reply; 37+ messages in thread
From: Bruce Richardson @ 2021-02-09 11:50 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: thomas, Ruifeng.Wang, Honnappa.Nagarahalli, jerinjacobk,
	hemant.agrawal, ferruh.yigit, aboyer, 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š <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
> 

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [PATCH v5] build: kni cross-compilation support
  2021-02-09 11:50         ` Bruce Richardson
@ 2021-02-09 12:07           ` Juraj Linkeš
  2021-02-09 12:39             ` Bruce Richardson
  0 siblings, 1 reply; 37+ messages in thread
From: Juraj Linkeš @ 2021-02-09 12:07 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: thomas, Ruifeng.Wang, Honnappa.Nagarahalli, jerinjacobk,
	hemant.agrawal, ferruh.yigit, aboyer, dev



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


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [PATCH v5] build: kni cross-compilation support
  2021-02-09 12:07           ` Juraj Linkeš
@ 2021-02-09 12:39             ` Bruce Richardson
  0 siblings, 0 replies; 37+ messages in thread
From: Bruce Richardson @ 2021-02-09 12:39 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: thomas, Ruifeng.Wang, Honnappa.Nagarahalli, jerinjacobk,
	hemant.agrawal, ferruh.yigit, aboyer, dev

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.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [dpdk-dev] [PATCH v6] build: kni cross-compilation support
  2021-02-09  8:47       ` [dpdk-dev] [PATCH v5] " Juraj Linkeš
  2021-02-09 11:50         ` Bruce Richardson
@ 2021-02-11 12:59         ` Juraj Linkeš
  2021-03-09  8:47           ` Juraj Linkeš
  2021-03-15 22:45           ` Thomas Monjalon
  1 sibling, 2 replies; 37+ messages in thread
From: Juraj Linkeš @ 2021-02-11 12:59 UTC (permalink / raw)
  To: bruce.richardson, thomas, Ruifeng.Wang, Honnappa.Nagarahalli,
	jerinjacobk, hemant.agrawal, ferruh.yigit, aboyer
  Cc: dev, Juraj Linkeš

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>
---
 kernel/linux/kni/meson.build |  8 ++--
 kernel/linux/meson.build     | 90 ++++++++++++++++++++++++++++++------
 meson_options.txt            |  2 +-
 3 files changed, 82 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..7b15e0fd44 100644
--- a/kernel/linux/meson.build
+++ b/kernel/linux/meson.build
@@ -3,25 +3,89 @@
 
 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()
+		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..3b8c5d316d 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 or $kernel_dir/build. Modules will be installed in /lib/modules.')
 option('machine', type: 'string', value: 'native',
 	description: 'set the target machine type')
 option('max_ethports', type: 'integer', value: 32,
-- 
2.20.1


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [PATCH v6] build: kni cross-compilation support
  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
  1 sibling, 1 reply; 37+ messages in thread
From: Juraj Linkeš @ 2021-03-09  8:47 UTC (permalink / raw)
  To: Juraj Linkeš,
	bruce.richardson, thomas, Ruifeng.Wang, Honnappa.Nagarahalli,
	jerinjacobk, hemant.agrawal, ferruh.yigit, aboyer
  Cc: dev

Hi Folks,

Does anyone have any comments? Is the patch ready? If so, please send acks or reviewed-by, thanks.

Andrew, did you get a chance to test the patch?

Juraj

> -----Original Message-----
> From: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Sent: Thursday, February 11, 2021 2:00 PM
> To: bruce.richardson@intel.com; 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
> Cc: dev@dpdk.org; Juraj Linkeš <juraj.linkes@pantheon.tech>
> Subject: [PATCH v6] 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>
> Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  kernel/linux/kni/meson.build |  8 ++--
>  kernel/linux/meson.build     | 90 ++++++++++++++++++++++++++++++------
>  meson_options.txt            |  2 +-
>  3 files changed, 82 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..7b15e0fd44 100644
> --- a/kernel/linux/meson.build
> +++ b/kernel/linux/meson.build
> @@ -3,25 +3,89 @@
> 
>  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()
> +		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..3b8c5d316d 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 or $kernel_dir/build. Modules will be installed
> +in /lib/modules.')
>  option('machine', type: 'string', value: 'native',
>  	description: 'set the target machine type')  option('max_ethports', type:
> 'integer', value: 32,
> --
> 2.20.1


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [PATCH v6] build: kni cross-compilation support
  2021-03-09  8:47           ` Juraj Linkeš
@ 2021-03-09 16:26             ` Andrew Boyer
  0 siblings, 0 replies; 37+ messages in thread
From: Andrew Boyer @ 2021-03-09 16:26 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: bruce.richardson, thomas, Ruifeng.Wang, Honnappa.Nagarahalli,
	jerinjacobk, hemant.agrawal, ferruh.yigit, dev

No, I am sorry, I have not tested it. I was able to remove rte_kni from our requirements so this is no longer a blocker for us.

-Andrew

> On Mar 9, 2021, at 3:47 AM, Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> 
> Hi Folks,
> 
> Does anyone have any comments? Is the patch ready? If so, please send acks or reviewed-by, thanks.
> 
> Andrew, did you get a chance to test the patch?
> 
> Juraj
> 
>> -----Original Message-----
>> From: Juraj Linkeš <juraj.linkes@pantheon.tech>
>> Sent: Thursday, February 11, 2021 2:00 PM
>> To: bruce.richardson@intel.com; 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
>> Cc: dev@dpdk.org; Juraj Linkeš <juraj.linkes@pantheon.tech>
>> Subject: [PATCH v6] 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>
>> Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
>> ---
>> kernel/linux/kni/meson.build |  8 ++--
>> kernel/linux/meson.build     | 90 ++++++++++++++++++++++++++++++------
>> meson_options.txt            |  2 +-
>> 3 files changed, 82 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..7b15e0fd44 100644
>> --- a/kernel/linux/meson.build
>> +++ b/kernel/linux/meson.build
>> @@ -3,25 +3,89 @@
>> 
>> 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()
>> +		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..3b8c5d316d 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 or $kernel_dir/build. Modules will be installed
>> +in /lib/modules.')
>> option('machine', type: 'string', value: 'native',
>> 	description: 'set the target machine type')  option('max_ethports', type:
>> 'integer', value: 32,
>> --
>> 2.20.1
> 


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [dpdk-dev] [PATCH v6] build: kni cross-compilation support
  2021-02-11 12:59         ` [dpdk-dev] [PATCH v6] " Juraj Linkeš
  2021-03-09  8:47           ` Juraj Linkeš
@ 2021-03-15 22:45           ` Thomas Monjalon
  1 sibling, 0 replies; 37+ messages in thread
From: Thomas Monjalon @ 2021-03-15 22:45 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: bruce.richardson, Ruifeng.Wang, Honnappa.Nagarahalli,
	jerinjacobk, hemant.agrawal, ferruh.yigit, aboyer, dev

11/02/2021 13:59, Juraj Linkeš:
> 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>

Applied, thanks




^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2021-03-15 22:45 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-29 10:29 [dpdk-dev] [RFC PATCH v1] build: kni gcc cross-compilation support 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
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

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).