Re: [dpdk-dev] [PATCH v20 1/3] build: disable/enable drivers in Arm builds

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

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Wednesday, April 14, 2021 3:28 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: Ruifeng.Wang@arm.com; Honnappa.Nagarahalli@arm.com;
> Phil.Yang@arm.com; vcchunga@amazon.com; Dharmik.Thakkar@arm.com;
> jerinjacobk@gmail.com; hemant.agrawal@nxp.com;
> ajit.khaparde@broadcom.com; ferruh.yigit@intel.com; aboyer@pensando.io;
> dev@dpdk.org
> Subject: Re: [PATCH v20 1/3] build: disable/enable drivers in Arm builds
> 
> On Wed, Apr 14, 2021 at 02:58:36PM +0200, Juraj Linkeš wrote:
> > Add support for enabling or disabling drivers for Arm cross build. Do
> > not implement any enable/disable lists yet.
> >
> > Enabling drivers is useful when building for an SoC where we only want
> > to build a few drivers. That way the list won't be too long.
> >
> > Similarly, disabling drivers is useful when we want to disable only a
> > few drivers.
> >
> > Both of these are advantageous mainly in aarch64 -> aarch64 (or arch
> > -> same arch) builds, where the build machine may have the required
> > driver dependencies, yet we don't want to build drivers for a specific SoC.
> >
> > If enable_drivers is a non-empty list, build only those drivers,
> > otherwise build all drivers and add them to enable_drivers.  If
> > disable_drivers is non-empty list, build all drivers specified in
> > enable_drivers except those in disable_drivers.
> >
> > There are two drivers, bus/pci and bus/vdev, which break the build if
> > not enabled. Address this by always enabling these if the user
> > disables them or doesn't specify in their allowlist.
> >
> > Also remove the old Makefile arm configuration options which don't do
> > anything in Meson.
> >
> > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> 
> Please see one comment inline below. Otherwise LGTM.
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> > ---
> >  buildtools/list-dir-globs.py                  |  7 ++--
> >  config/arm/meson.build                        |  4 ---
> >  config/meson.build                            |  4 +++
> >  .../linux_gsg/cross_build_dpdk_for_arm64.rst  |  8 +++++
> >  drivers/common/qat/meson.build                |  4 +--
> >  drivers/meson.build                           | 32 ++++++++++++++++---
> >  meson_options.txt                             |  2 ++
> >  7 files changed, 48 insertions(+), 13 deletions(-)
> >
> > diff --git a/buildtools/list-dir-globs.py
> > b/buildtools/list-dir-globs.py index 80b5e801f2..911e267335 100755
> > --- a/buildtools/list-dir-globs.py
> > +++ b/buildtools/list-dir-globs.py
> > @@ -14,6 +14,7 @@
> >                      os.getenv('MESON_SUBDIR', '.'))
> >
> >  for path in sys.argv[1].split(','):
> > -    for p in iglob(os.path.join(root, path)):
> > -        if os.path.isdir(p):
> > -            print(os.path.relpath(p))
> > +    if path:
> > +        for p in iglob(os.path.join(root, path)):
> > +            if os.path.isdir(p):
> > +                print(os.path.relpath(p))
> > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > aaed89bd5b..2984ae2db3 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -17,9 +17,6 @@ flags_common = [
> >  	#	['RTE_ARM64_MEMCPY_ALIGN_MASK', 0xF],
> >  	#	['RTE_ARM64_MEMCPY_STRICT_ALIGN', false],
> >
> > -	['RTE_NET_FM10K', false],
> > -	['RTE_NET_AVP', false],
> > -
> >  	['RTE_SCHED_VECTOR', false],
> >  	['RTE_ARM_USE_WFE', false],
> >  	['RTE_ARCH_ARM64', true],
> > @@ -126,7 +123,6 @@ implementer_cavium = {
> >  				['RTE_MACHINE', '"octeontx2"'],
> >  				['RTE_ARM_FEATURE_ATOMICS', true],
> >  				['RTE_USE_C11_MEM_MODEL', true],
> > -				['RTE_EAL_IGB_UIO', false],
> >  				['RTE_MAX_LCORE', 36],
> >  				['RTE_MAX_NUMA_NODES', 1]
> >  			]
> > diff --git a/config/meson.build b/config/meson.build index
> > 3268cf6804..6e6ef8c0e1 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -63,6 +63,10 @@ if not is_windows
> >  			pmd_subdir_opt)
> >  endif
> >
> > +# init disable/enable driver lists that will be populated in
> > +different places disable_drivers = ''
> > +enable_drivers = ''
> > +
> >  # set the machine type and cflags for it  if meson.is_cross_build()
> >  	machine = host_machine.cpu()
> > diff --git a/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
> > b/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
> > index faaf24b95b..9b9e8ef704 100644
> > --- a/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
> > +++ b/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
> > @@ -234,3 +234,11 @@ There are other options you may specify in a cross
> file to tailor the build::
> >        numa = false        # set to false to force building for a non-NUMA system
> >           # if not set or set to true, the build system will build for a NUMA
> >           # system only if libnuma is installed
> > +
> > +      disable_drivers = 'bus/dpaa,crypto/*'  # add disabled drivers
> > +         # valid values are dir/subdirs in the drivers directory
> > +         # wildcards are allowed
> > +
> > +      enable_drivers = 'common/*,bus/*'  # build only these drivers
> > +         # valid values are dir/subdirs in the drivers directory
> > +         # wildcards are allowed
> > diff --git a/drivers/common/qat/meson.build
> > b/drivers/common/qat/meson.build index 3ad7dd5017..fe278f734f 100644
> > --- a/drivers/common/qat/meson.build
> > +++ b/drivers/common/qat/meson.build
> > @@ -14,13 +14,13 @@ qat_compress = true  qat_compress_path =
> > 'compress/qat'
> >  qat_compress_relpath = '../../' + qat_compress_path
> >
> > -if disabled_drivers.contains(qat_crypto_path)
> > +if disable_drivers.contains(qat_crypto_path)
> >  	qat_crypto = false
> >  	dpdk_drvs_disabled += qat_crypto_path
> >  	set_variable(qat_crypto_path.underscorify() + '_disable_reason',
> >  			'Explicitly disabled via build config')  endif -if
> > disabled_drivers.contains(qat_compress_path)
> > +if disable_drivers.contains(qat_compress_path)
> >  	qat_compress = false
> >  	dpdk_drvs_disabled += qat_compress_path
> >  	set_variable(qat_compress_path.underscorify() + '_disable_reason',
> > diff --git a/drivers/meson.build b/drivers/meson.build index
> > 45af8749da..00fe70452d 100644
> > --- a/drivers/meson.build
> > +++ b/drivers/meson.build
> > @@ -20,8 +20,25 @@ subdirs = [
> >  	'baseband', # depends on common and bus.
> >  ]
> >
> > -disabled_drivers = run_command(list_dir_globs, get_option('disable_drivers'),
> > -		).stdout().split()
> > +if meson.is_cross_build()
> > +	disable_drivers += ',' + meson.get_cross_property('disable_drivers', [])
> > +	enable_drivers += ',' + meson.get_cross_property('enable_drivers',
> > +[])
> 
> Should the fallbacks here not be empty strings rather than empty values?
> 

That's right, I overlooked this, thanks. I'll fix this right away.

> > +endif
> > +
> > +# add cmdline disabled drivers and meson disabled drivers together
> > +disable_drivers += ',' + get_option('disable_drivers')
> > +disable_drivers = run_command(list_dir_globs,
> > +disable_drivers).stdout().split()
> > +
> > +# add cmdline enabled drivers and meson enabled drivers together
> > +enable_drivers = ',' + get_option('enable_drivers') enable_drivers =
> > +run_command(list_dir_globs, enable_drivers).stdout().split() if
> > +enable_drivers.length() == 0
> > +	enable_drivers = run_command(list_dir_globs, '*/*').stdout().split()
> > +endif
> > +
> > +# these drivers must always be enabled, otherwise the build breaks
> > +always_enable = ['bus/pci', 'bus/vdev'] enable_drivers +=
> > +always_enable
> >
> >  default_cflags = machine_args
> >  default_cflags += ['-DALLOW_EXPERIMENTAL_API'] @@ -75,9 +92,16 @@
> > foreach subpath:subdirs
> >  		ext_deps = []
> >  		pkgconfig_extra_libs = []
> >
> > -		if disabled_drivers.contains(drv_path)
> > +		if not enable_drivers.contains(drv_path)
> >  			build = false
> > -			reason = 'explicitly disabled via build config'
> > +			reason = 'not in enabled drivers build config'
> > +		elif disable_drivers.contains(drv_path)
> > +			if always_enable.contains(drv_path)
> > +				message('Driver @0@ cannot be disabled, not
> disabling.'.format(drv_path))
> > +			else
> > +				build = false
> > +				reason = 'explicitly disabled via build config'
> > +			endif
> >  		else
> >  			# pull in driver directory which should update all the
> local variables
> >  			subdir(drv_path)
> > diff --git a/meson_options.txt b/meson_options.txt index
> > fa207cbada..e1dc351620 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -10,6 +10,8 @@ option('drivers_install_subdir', type: 'string', value:
> 'dpdk/pmds-<VERSION>',
> >  	description: 'Subdirectory of libdir where to install PMDs. Defaults
> > to using a versioned subdirectory.')  option('enable_docs', type: 'boolean',
> value: false,
> >  	description: 'build documentation')
> > +option('enable_drivers', type: 'string', value: '',
> > +	description: 'Comma-separated list of drivers to build. If
> > +unspecified, build all drivers.')
> >  option('enable_driver_sdk', type: 'boolean', value: false,
> >  	description: 'Install headers to build drivers.')
> > option('enable_kmods', type: 'boolean', value: false,
> > --
> > 2.20.1
> >