Re: [dpdk-dev] [RFC PATCH v1] build: add platform meson option

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

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Thursday, November 26, 2020 5:03 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: thomas@monjalon.net; Honnappa.Nagarahalli@arm.com; dev@dpdk.org
> Subject: Re: [RFC PATCH v1] build: add platform meson option
> 
> On Thu, Nov 26, 2020 at 04:47:29PM +0100, Juraj Linkeš wrote:
> > The current meson option 'machine' should only specify the ISA, which
> > is not sufficient for Arm, where setting ISA implies other setting as well.
> > Add a new meson option, 'platform', which differentiates the type of
> > the build (native/generic) and sets machine accordingly, unless the
> > user chooses to override it.
> >
> > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > ---
> >  config/arm/meson.build |  2 +-
> >  config/meson.build     | 14 +++++++++++++-
> >  meson_options.txt      |  6 ++++--
> >  3 files changed, 18 insertions(+), 4 deletions(-)
> >
> > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > 42b4e43c7..ac680956f 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -6,7 +6,7 @@
> >  march_opt = '-march=@0@'.format(machine)
> >
> >  arm_force_native_march = false
> > -arm_force_default_march = (machine == 'default')
> > +arm_force_default_march = (platform == 'generic')
> >
> >  flags_common_default = [
> >  	# Accelarate rte_memcpy. Be sure to run unit test
> > (memcpy_perf_autotest) diff --git a/config/meson.build
> > b/config/meson.build index c02802c18..41d32e63e 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -63,6 +63,8 @@ if not is_windows
> >  			pmd_subdir_opt)
> >  endif
> >
> > +platform = get_option('platform')
> > +
> >  # set the machine type and cflags for it  if meson.is_cross_build()
> >  	machine = host_machine.cpu()
> > @@ -70,13 +72,23 @@ else
> >  	machine = get_option('machine')
> >  endif
> >
> > +if platform == 'native'
> > +	if machine == 'auto'
> > +		machine = 'native'
> > +	endif
> > +elif platform == 'generic'
> > +	if machine == 'auto'
> > +		machine = 'default'
> > +	endif
> > +endif
> > +
> > +if machine == 'default'
> >  # machine type 'default' is special, it defaults to the per arch
> > agreed common  # minimal baseline needed for DPDK.
> >  # That might not be the most optimized, but the most portable version
> > while  # still being able to support the CPU features required for DPDK.
> >  # This can be bumped up by the DPDK project, but it can never be an
> > # invariant like 'native'
> > -if machine == 'default'
> >  	if host_machine.cpu_family().startswith('x86')
> >  		# matches the old pre-meson build systems default
> >  		machine = 'corei7'
> > diff --git a/meson_options.txt b/meson_options.txt index
> > e384e6dbb..1a5e47fd3 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -20,14 +20,16 @@ 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.')
> >  option('lib_musdk_dir', type: 'string', value: '',
> >  	description: 'path to the MUSDK library installation directory')
> > -option('machine', type: 'string', value: 'native',
> > -	description: 'set the target machine type')
> > +option('machine', type: 'string', value: 'auto',
> > +	description: 'set the target machine type/ISA')
> >  option('max_ethports', type: 'integer', value: 32,
> >  	description: 'maximum number of Ethernet devices')
> > option('max_lcores', type: 'integer', value: 128,
> >  	description: 'maximum number of cores/threads supported by EAL')
> > option('max_numa_nodes', type: 'integer', value: 4,
> >  	description: 'maximum number of NUMA nodes supported by EAL')
> > +option('platform', type: 'string', value: 'generic',
> > +	description: 'Platform to build for, either "native" or "generic".')
> 
> As well as this short description option, I think we need more comprehensive
> coverage of this option in the docs.

Where should this be documented? In doc/guides/linux_gsg/build_dpdk.rst or somewhere else?

> Presumably for ARM systems this will have
> other options for various SOC's rather than just generic/native?
> 

Yes, I'm planning on using the platform option for this. Since we've separated the changes into their own patch sets, only this small change is in this patch set. It'll affect how we're doing the automatic numa/core detection as well as how we're choosic which Arm SoC to build for, but those changes are in different patch sets.

I mainly want feedback on naming - I like 'platform' and I don't think we need to rename 'machine', maybe just document it better - and on the overall idea. I think this is in line with what you had in mind, right?

> /Bruce