[v12,11/14] build: disable Arm drivers

Message ID 1605277875-13625-12-git-send-email-juraj.linkes@pantheon.tech (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series Arm build options rework |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Juraj Linkeš Nov. 13, 2020, 2:31 p.m. UTC
  A few options that disabled drivers in the old makefiles were improperly
ported to the meson build system. Fix this by adding a to the list of
disabled drivers, similarly how the command line option works. Remove
unneeded driver options ported from the old makefile system.
Add support for removing drivers for cross builds.

Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 config/arm/arm64_armada_linux_gcc                   | 1 +
 config/arm/meson.build                              | 7 +++----
 doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst | 4 ++++
 drivers/meson.build                                 | 6 +++++-
 meson.build                                         | 1 +
 5 files changed, 14 insertions(+), 5 deletions(-)
  

Comments

Thomas Monjalon Nov. 16, 2020, 7:28 a.m. UTC | #1
What do you mean by "disable Arm drivers"?
What are "Arm drivers"

13/11/2020 15:31, Juraj Linkeš:
> A few options that disabled drivers in the old makefiles were improperly
> ported to the meson build system. Fix this by adding a to the list of
> disabled drivers, similarly how the command line option works. Remove
> unneeded driver options ported from the old makefile system.
> Add support for removing drivers for cross builds.
> 
> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
[...]
> -	['RTE_NET_FM10K', false],
> -	['RTE_NET_AVP', false],

Isn't it enabling drivers?

[...]
> +if meson.is_cross_build()
> +	disabled_drivers += meson.get_cross_property('disabled_drivers', [])
> +endif

Why is it specific to cross build?
  
Juraj Linkeš Nov. 16, 2020, 7:56 a.m. UTC | #2
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, November 16, 2020 8:29 AM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: bruce.richardson@intel.com; 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; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v12 11/14] build: disable Arm drivers
> 
> What do you mean by "disable Arm drivers"?
> What are "Arm drivers"
> 

These are the drivers that we're disableing for Arm builds. I'll change it to something more clearer like "disable drivers in Arm builds"

> 13/11/2020 15:31, Juraj Linkeš:
> > A few options that disabled drivers in the old makefiles were
> > improperly ported to the meson build system. Fix this by adding a to
> > the list of disabled drivers, similarly how the command line option
> > works. Remove unneeded driver options ported from the old makefile system.
> > Add support for removing drivers for cross builds.
> >
> > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> [...]
> > -	['RTE_NET_FM10K', false],
> > -	['RTE_NET_AVP', false],
> 
> Isn't it enabling drivers?
> 

These options don't do anything in the way the current meson build system is set up, since they'll be overwritten in drivers/meson.build. The order is, from meson.build (in the git root):
subdir('config')   # here we're setting arm specific flags
...
subdir('drivers')  # and here they'd be overwritten

> [...]
> > +if meson.is_cross_build()
> > +	disabled_drivers += meson.get_cross_property('disabled_drivers', [])
> > +endif
> 
> Why is it specific to cross build?
> 

I don't understand the question, what is "it"? This is adding support for disabling drivers in cross builds so I guess that part is specific to cross builds?
  
Thomas Monjalon Nov. 16, 2020, 8:22 a.m. UTC | #3
16/11/2020 08:56, Juraj Linkeš:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 
> > What do you mean by "disable Arm drivers"?
> > What are "Arm drivers"
> > 
> 
> These are the drivers that we're disableing for Arm builds. I'll change it to something more clearer like "disable drivers in Arm builds"
> 
> > 13/11/2020 15:31, Juraj Linkeš:
> > > A few options that disabled drivers in the old makefiles were
> > > improperly ported to the meson build system. Fix this by adding a to
> > > the list of disabled drivers, similarly how the command line option
> > > works. Remove unneeded driver options ported from the old makefile system.
> > > Add support for removing drivers for cross builds.
> > >
> > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> > > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > [...]
> > > -	['RTE_NET_FM10K', false],
> > > -	['RTE_NET_AVP', false],
> > 
> > Isn't it enabling drivers?
> > 
> 
> These options don't do anything in the way the current meson build system is set up, since they'll be overwritten in drivers/meson.build. The order is, from meson.build (in the git root):
> subdir('config')   # here we're setting arm specific flags
> ...
> subdir('drivers')  # and here they'd be overwritten

Should this change be in the same patch as disabling some drivers?
If yes, please explain in the commit log.


> > [...]
> > > +if meson.is_cross_build()
> > > +	disabled_drivers += meson.get_cross_property('disabled_drivers', [])
> > > +endif
> > 
> > Why is it specific to cross build?
> 
> I don't understand the question, what is "it"? This is adding support for disabling drivers in cross builds so I guess that part is specific to cross builds?

OK I understand.
  
Juraj Linkeš Nov. 16, 2020, 3:54 p.m. UTC | #4
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, November 16, 2020 9:23 AM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: bruce.richardson@intel.com; 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; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v12 11/14] build: disable Arm drivers
> 
> 16/11/2020 08:56, Juraj Linkeš:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > >
> > > What do you mean by "disable Arm drivers"?
> > > What are "Arm drivers"
> > >
> >
> > These are the drivers that we're disableing for Arm builds. I'll change it to
> something more clearer like "disable drivers in Arm builds"
> >
> > > 13/11/2020 15:31, Juraj Linkeš:
> > > > A few options that disabled drivers in the old makefiles were
> > > > improperly ported to the meson build system. Fix this by adding a
> > > > to the list of disabled drivers, similarly how the command line
> > > > option works. Remove unneeded driver options ported from the old
> makefile system.
> > > > Add support for removing drivers for cross builds.
> > > >
> > > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> > > > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > [...]
> > > > -	['RTE_NET_FM10K', false],
> > > > -	['RTE_NET_AVP', false],
> > >
> > > Isn't it enabling drivers?
> > >
> >
> > These options don't do anything in the way the current meson build system is
> set up, since they'll be overwritten in drivers/meson.build. The order is, from
> meson.build (in the git root):
> > subdir('config')   # here we're setting arm specific flags
> > ...
> > subdir('drivers')  # and here they'd be overwritten
> 
> Should this change be in the same patch as disabling some drivers?
> If yes, please explain in the commit log.
> 

I think so, the fix consist not only of adding support for disabling drivers, but also removing the parts that look like they're doing that but fail to do so (so, in essence replacing the faulty config with proper support). I'll update the commit message.

> 
> > > [...]
> > > > +if meson.is_cross_build()
> > > > +	disabled_drivers += meson.get_cross_property('disabled_drivers',
> > > > +[]) endif
> > >
> > > Why is it specific to cross build?
> >
> > I don't understand the question, what is "it"? This is adding support for
> disabling drivers in cross builds so I guess that part is specific to cross builds?
> 
> OK I understand.
> 
>
  
Thomas Monjalon Nov. 16, 2020, 8:35 p.m. UTC | #5
16/11/2020 16:54, Juraj Linkeš:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 16/11/2020 08:56, Juraj Linkeš:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > >
> > > > What do you mean by "disable Arm drivers"?
> > > > What are "Arm drivers"
> > > >
> > >
> > > These are the drivers that we're disableing for Arm builds. I'll change it to
> > something more clearer like "disable drivers in Arm builds"
> > >
> > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > A few options that disabled drivers in the old makefiles were
> > > > > improperly ported to the meson build system. Fix this by adding a
> > > > > to the list of disabled drivers, similarly how the command line
> > > > > option works. Remove unneeded driver options ported from the old
> > makefile system.
> > > > > Add support for removing drivers for cross builds.
> > > > >
> > > > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > > > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> > > > > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > > [...]
> > > > > -	['RTE_NET_FM10K', false],
> > > > > -	['RTE_NET_AVP', false],
> > > >
> > > > Isn't it enabling drivers?
> > > >
> > >
> > > These options don't do anything in the way the current meson build system is
> > set up, since they'll be overwritten in drivers/meson.build. The order is, from
> > meson.build (in the git root):
> > > subdir('config')   # here we're setting arm specific flags
> > > ...
> > > subdir('drivers')  # and here they'd be overwritten
> > 
> > Should this change be in the same patch as disabling some drivers?
> > If yes, please explain in the commit log.
> > 
> 
> I think so, the fix consist not only of adding support for disabling drivers, but also removing the parts that look like they're doing that but fail to do so (so, in essence replacing the faulty config with proper support). I'll update the commit message.

OK
  

Patch

diff --git a/config/arm/arm64_armada_linux_gcc b/config/arm/arm64_armada_linux_gcc
index 2ecc4604c..e365f61d0 100644
--- a/config/arm/arm64_armada_linux_gcc
+++ b/config/arm/arm64_armada_linux_gcc
@@ -18,3 +18,4 @@  implementer_id = '0x56'
 part_number = '0xd08'
 max_lcores = 16
 max_numa_nodes = 1
+disabled_drivers = ['bus/dpaa', 'bus/fslmc', 'common/dpaax']
diff --git a/config/arm/meson.build b/config/arm/meson.build
index 7945efb72..9b167267f 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -3,6 +3,9 @@ 
 # Copyright(c) 2017 Cavium, Inc
 # Copyright(c) 2020 PANTHEON.tech s.r.o.
 
+# disable Arm drivers for all builds
+disabled_drivers += ['net/avp', 'net/fm10k']
+
 # common flags to all aarch64 builds, with lowest priority
 flags_common = [
 	# Accelerate rte_memcpy. Be sure to run unit test (memcpy_perf_autotest)
@@ -17,9 +20,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],
@@ -127,7 +127,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/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst b/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
index 4e65b271c..210ad4508 100644
--- a/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
+++ b/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
@@ -143,3 +143,7 @@  There are other options you may specify in a cross file to tailor the build::
    Supported extra configuration
       max_numa_nodes = n  # will set RTE_MAX_NUMA_NODES
       max_lcores = n      # will set RTE_MAX_LCORE
+
+      disabled_drivers = ['bus/dpaa', 'crypto']  # add disabled drivers
+         # valid values are directories (optionally with their subdirs)
+         # in the drivers directory
diff --git a/drivers/meson.build b/drivers/meson.build
index 6b50f7238..21a296b55 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -18,9 +18,13 @@  subdirs = [
 	'baseband', # depends on common and bus.
 ]
 
-disabled_drivers = run_command(list_dir_globs, get_option('disable_drivers'),
+disabled_drivers += run_command(list_dir_globs, get_option('disable_drivers'),
 		).stdout().split()
 
+if meson.is_cross_build()
+	disabled_drivers += meson.get_cross_property('disabled_drivers', [])
+endif
+
 default_cflags = machine_args
 default_cflags += ['-DALLOW_EXPERIMENTAL_API']
 default_cflags += ['-DALLOW_INTERNAL_API']
diff --git a/meson.build b/meson.build
index 45d974cd2..fcc8931f0 100644
--- a/meson.build
+++ b/meson.build
@@ -21,6 +21,7 @@  dpdk_drivers = []
 dpdk_extra_ldflags = []
 dpdk_libs_disabled = []
 dpdk_drvs_disabled = []
+disabled_drivers = []
 abi_version_file = files('ABI_VERSION')
 
 if host_machine.cpu_family().startswith('x86')