-----Original Message-----
From: Honnappa Nagarahalli <redacted>
Sent: Thursday, September 24, 2020 6:19 AM
To: Juraj Linkeš <redacted>; bruce.richardson@intel.com;
Ruifeng Wang [off-list ref]; Phil Yang [off-list ref];
vcchunga@amazon.com; Dharmik Thakkar [off-list ref];
jerinjacobk@gmail.com; hemant.agrawal@nxp.com
Cc: dev@dpdk.org; nd <redacted>; Honnappa Nagarahalli
[off-list ref]; nd [off-list ref]
Subject: RE: [RFC PATCH 1/2] build: rework Arm build options

Thanks Juraj for the quick RFC. Few comments inline.

<snip>

quoted

The current system can identify only the implementer and part number
of the arm device we're targeting, which is enough to identify the target CPU.
However, even the full MIDR information is not enough to identify the
SoC we're targeting.

Expand the "machine" meson variable to allow specifying the target arm SoC.
The SoC identification implies the CPU, so drop the current MIDR based
CPU identification in favor of user input, which is the only way to
identify the target SoC. This implies that native builds are not
possible. Instead do a default (non-optimized, but executable on any
aarch64 device) build.

Also use the machine custom property in cross files for cross builds.

An example of where the current system is insufficient is the cortex-a72 CPU.
These cores are used in a variety of SoC from Broadcomm, NXP, Huawei
and others. The MIDR information would only identify the core, but
there's not way to identify the SoC to fine tune the build to it.

Signed-off-by: Juraj Linkeš <redacted>
---
 config/arm/arm64_armv8_linux_gcc              |  28 ----
 config/arm/default/arm64_armv8_linux_gcc      |  23 +++
 config/arm/default/meson.build                |   8 +
 config/arm/meson.build                        | 139 +++++++++++-------
 .../{ => thunderx}/arm64_thunderx_linux_gcc   |   2 +-
 config/arm/thunderx/meson.build               |  18 +++
 .../thunderxt88/arm64_thunderxt88_linux_gcc   |  16 ++
 config/arm/thunderxt88/meson.build            |  18 +++
 config/meson.build                            |   9 +-
 9 files changed, 177 insertions(+), 84 deletions(-)  delete mode
100644 config/arm/arm64_armv8_linux_gcc  create mode 100644
config/arm/default/arm64_armv8_linux_gcc
 create mode 100644 config/arm/default/meson.build  rename
config/arm/{ => thunderx}/arm64_thunderx_linux_gcc (92%)  create mode
100644 config/arm/thunderx/meson.build  create mode 100644
config/arm/thunderxt88/arm64_thunderxt88_linux_gcc
 create mode 100644 config/arm/thunderxt88/meson.build

diff --git a/config/arm/arm64_armv8_linux_gcc

b/config/arm/arm64_armv8_linux_gcc
deleted file mode 100644
index 88f0ff9da..000000000

--- a/config/arm/arm64_armv8_linux_gcc
+++ /dev/null

@@ -1,28 +0,0 @@
-[binaries]
-c = 'aarch64-linux-gnu-gcc'
-cpp = 'aarch64-linux-gnu-cpp'
-ar = 'aarch64-linux-gnu-gcc-ar'
-strip = 'aarch64-linux-gnu-strip'
-pkgconfig = 'aarch64-linux-gnu-pkg-config'
-pcap-config = ''
-
-[host_machine]
-system = 'linux'
-cpu_family = 'aarch64'
-cpu = 'armv8-a'
-endian = 'little'
-
-[properties]
-implementor_id = 'generic'
-
-# Valid options for Arm's implementor_pn:
-# 'default': valid for all armv8-a architectures (default value)
-# '0xd03':   cortex-a53
-# '0xd04':   cortex-a35
-# '0xd05':   cortex-a55
-# '0xd07':   cortex-a57
-# '0xd08':   cortex-a72
-# '0xd09':   cortex-a73
-# '0xd0a':   cortex-a75
-# '0xd0b':   cortex-a76
-implementor_pn = 'default'

diff --git a/config/arm/default/arm64_armv8_linux_gcc

b/config/arm/default/arm64_armv8_linux_gcc
new file mode 100644
index 000000000..56efd7a05

--- /dev/null
+++ b/config/arm/default/arm64_armv8_linux_gcc

@@ -0,0 +1,23 @@
+[binaries]
+c = 'aarch64-linux-gnu-gcc'
+cpp = 'aarch64-linux-gnu-cpp'
+ar = 'aarch64-linux-gnu-gcc-ar'
+strip = 'aarch64-linux-gnu-strip'
+pkgconfig = 'aarch64-linux-gnu-pkg-config'
+pcap-config = ''
+
+[host_machine]
+system = 'linux'
+cpu_family = 'aarch64'
+cpu = 'armv8-a'
+endian = 'little'
+
+[properties]
+# Valid values for the machine property are the soc names identified
+by # directories in config/arm # Possible suffixes for socs:
+# -mn-kc
+# where m is the number of numa nodes and k is the number of lcores #
+e.g. for thunderx soc the possible values are "thunderx" or
+"thunderx-1n-

48c"

Is it required to talk about the suffixes? I think the statement "Valid values for
the machine property are the soc names identified by directories in config/arm"
should be sufficient.

Yes, this is not the right place for this, so a shorted explanation is enough.

quoted

+# the suffixes themselves need to be supported in
+config/arm/<soc_dir>/meson.build machine = 'default'

IMO, "default" does not convey much information. Is it possible to change it
without a deprecation notice? "generic" or "armv8" conveys more information.
Any opinions from others?

Maybe we can have multiple labels mean the same thing - default, generic, armv8 or even generic-armv8 (that's the most descriptive name IMO) could all mean the generic arm build executable on all aarch64 SoCs. That many names is probably confusing, but sticking to two (default and one more) seems feasible.

What does everyone else think?

quoted

diff --git a/config/arm/default/meson.build

b/config/arm/default/meson.build new file mode 100644 index
000000000..5714d0084

--- /dev/null
+++ b/config/arm/default/meson.build

@@ -0,0 +1,8 @@
+flags = [
+	['RTE_MACHINE', '"armv8a"'],
+	['RTE_MAX_LCORE', 256],
+	['RTE_USE_C11_MEM_MODEL', true],
+	['RTE_CACHE_LINE_SIZE', 128]
+]
+
+machine_args = ['-march=armv8-a+crc']

diff --git a/config/arm/meson.build b/config/arm/meson.build index

8728051d5..05b790f40 100644

--- a/config/arm/meson.build
+++ b/config/arm/meson.build

@@ -2,11 +2,11 @@
 # Copyright(c) 2017 Intel Corporation.
 # Copyright(c) 2017 Cavium, Inc

-# for checking defines we need to use the correct compiler flags
-march_opt = '-march=@0@'.format(machine)
-
-arm_force_native_march = false
-arm_force_default_march = (machine == 'default')
+if machine == 'native'
+	# arm doesn't support a native build - user input of soc is required
+	# default to the portable build instead of native
+	machine = 'default'

Do we need a message here to the user?

Better to have one, good catch.

quoted

+endif

 flags_common_default = [
 	# Accelarate rte_memcpy. Be sure to run unit test
(memcpy_perf_autotest) @@ -131,6 +131,7 @@ impl_dpaa = ['NXP DPAA',
flags_dpaa, machine_args_generic]
dpdk_conf.set('RTE_FORCE_INTRINSICS',
1)

 if not dpdk_conf.get('RTE_ARCH_64')
+	# armv7 build
 	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
 	dpdk_conf.set('RTE_ARCH_ARM', 1)
 	dpdk_conf.set('RTE_ARCH_ARMv7', 1)

@@ -138,41 +139,9 @@ if not dpdk_conf.get('RTE_ARCH_64')
 	# mk/machine/armv7a/rte.vars.mk sets it too
 	machine_args += '-mfpu=neon'
 else
+	# aarch64 build
 	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128)
 	dpdk_conf.set('RTE_ARCH_ARM64', 1)
-
-	machine = []
-	cmd_generic = ['generic', '', '', 'default', '']
-	cmd_output = cmd_generic # Set generic by default
-	machine_args = [] # Clear previous machine args
-	if arm_force_default_march and not meson.is_cross_build()
-		machine = impl_generic
-		impl_pn = 'default'
-	elif not meson.is_cross_build()
-		# The script returns ['Implementer', 'Variant', 'Architecture',
-		# 'Primary Part number', 'Revision']
-		detect_vendor = find_program(join_paths(
-				meson.current_source_dir(),

'armv8_machine.py'))
-		cmd = run_command(detect_vendor.path())
-		if cmd.returncode() == 0
-			cmd_output = cmd.stdout().to_lower().strip().split(' ')
-		endif
-		# Set to generic if variable is not found
-		machine = get_variable('impl_' + cmd_output[0], ['generic'])
-		if machine[0] == 'generic'
-			machine = impl_generic
-			cmd_output = cmd_generic
-		endif
-		impl_pn = cmd_output[3]
-		if arm_force_native_march == true
-			impl_pn = 'native'
-		endif
-	else
-		impl_id = meson.get_cross_property('implementor_id',
'generic')
-		impl_pn = meson.get_cross_property('implementor_pn',
'default')
-		machine = get_variable('impl_' + impl_id)
-	endif
-
 	# Apply Common Defaults. These settings may be overwritten by
machine
 	# settings later.
 	foreach flag: flags_common_default

@@ -181,30 +150,89 @@ else
 		endif
 	endforeach

-	message('Implementer : ' + machine[0])
-	foreach flag: machine[1]
+	soc = machine.split('-')[0]
+	# TODO make sure the subdir exists
+	subdir(soc)
+
+	if soc != 'default'
+		# combine common and soc specific flags
+		# TODO check that soc is in the dict
+		flags = flags['common'] + flags[soc]
+	endif
+
+	foreach flag: flags
 		if flag.length() > 0
 			dpdk_conf.set(flag[0], flag[1])
 		endif
 	endforeach

-	foreach marg: machine[2]
-		if marg[0] == impl_pn
-			foreach flag: marg[1]
-				if cc.has_argument(flag)
-					machine_args += flag
-				endif
-			endforeach
-			# Apply any extra machine specific flags.
-			foreach flag: marg.get(2, flags_default_extra)
-				if flag.length() > 0
-					dpdk_conf.set(flag[0], flag[1])
-				endif
-			endforeach
+	verified_machine_args = []
+
+	foreach flag: machine_args
+		if cc.has_argument(flag)
+			verified_machine_args += flag
+		else
+			message('Unsupported machine flag "@0@",

ignoring'.format(flag))
 		endif
 	endforeach
+	machine_args = verified_machine_args
+
+#	machine = []
+#	cmd_generic = ['generic', '', '', 'default', '']
+#	cmd_output = cmd_generic # Set generic by default
+#	machine_args = [] # Clear previous machine args
+#	if arm_force_default_march and not meson.is_cross_build()
+#		machine = impl_generic
+#		impl_pn = 'default'
+#	elif not meson.is_cross_build()

Isn't this for native build? Do we still need this?

Yes, I should've removed this instead of commenting it out.

quoted

+#		# The script returns ['Implementer', 'Variant', 'Architecture',
+#		# 'Primary Part number', 'Revision']
+#		detect_vendor = find_program(join_paths(
+#				meson.current_source_dir(),
'armv8_machine.py'))
+#		cmd = run_command(detect_vendor.path())
+#		if cmd.returncode() == 0
+#			cmd_output = cmd.stdout().to_lower().strip().split(' ')
+#		endif
+#		# Set to generic if variable is not found
+#		machine = get_variable('impl_' + cmd_output[0], ['generic'])
+#		if machine[0] == 'generic'
+#			machine = impl_generic
+#			cmd_output = cmd_generic
+#		endif
+#		impl_pn = cmd_output[3]
+#		if arm_force_native_march == true
+#			impl_pn = 'native'
+#		endif
+#	else
+#		impl_id = meson.get_cross_property('implementor_id',
'generic')
+#		impl_pn = meson.get_cross_property('implementor_pn',
'default')
+#		machine = get_variable('impl_' + impl_id)
+#	endif
+#
+#	message('Implementer : ' + machine[0])
+#	foreach flag: machine[1]
+#		if flag.length() > 0
+#			dpdk_conf.set(flag[0], flag[1])
+#		endif
+#	endforeach
+#
+#	foreach marg: machine[2]
+#		if marg[0] == impl_pn
+#			foreach flag: marg[1]
+#				if cc.has_argument(flag)
+#					machine_args += flag
+#				endif
+#			endforeach
+#			# Apply any extra machine specific flags.
+#			foreach flag: marg.get(2, flags_default_extra)
+#				if flag.length() > 0
+#					dpdk_conf.set(flag[0], flag[1])
+#				endif
+#			endforeach
+#		endif
+#	endforeach
 endif
-message(machine_args)
+message('Using machine args: @0@'.format(machine_args))

 if (cc.get_define('__ARM_NEON', args: machine_args) != '' or
     cc.get_define('__aarch64__', args: machine_args) != '') @@ -225,3
+253,6 @@ if cc.get_define('__ARM_FEATURE_CRYPTO', args: machine_args)

!= ''

quoted

 	compile_time_cpuflags += ['RTE_CPUFLAG_AES',

'RTE_CPUFLAG_PMULL',

quoted

 	'RTE_CPUFLAG_SHA1', 'RTE_CPUFLAG_SHA2']  endif
+
+message('dpdk conf options after arm stuff: @0@'.format(dpdk_conf))
+

diff --git a/config/arm/arm64_thunderx_linux_gcc

b/config/arm/thunderx/arm64_thunderx_linux_gcc
similarity index 92%
rename from config/arm/arm64_thunderx_linux_gcc
rename to config/arm/thunderx/arm64_thunderx_linux_gcc
index 6572ab615..e53a86e8f 100644

--- a/config/arm/arm64_thunderx_linux_gcc
+++ b/config/arm/thunderx/arm64_thunderx_linux_gcc

@@ -13,4 +13,4 @@ cpu = 'armv8-a'
 endian = 'little'

 [properties]
-implementor_id = '0x43'
+machine = 'thunderx'

diff --git a/config/arm/thunderx/meson.build

b/config/arm/thunderx/meson.build new file mode 100644 index
000000000..873d79a64

--- /dev/null
+++ b/config/arm/thunderx/meson.build

@@ -0,0 +1,18 @@
+flags = {
+	'common': [
+		['RTE_CACHE_LINE_SIZE', 128],
+		['RTE_MAX_VFIO_GROUPS', 128],
+		['RTE_MACHINE', '"thunderx"'],
+		['RTE_USE_C11_MEM_MODEL', false]
+	],
+	'thunderx': [
+		['RTE_MAX_NUMA_NODES', 2],
+		['RTE_MAX_LCORE', 96]
+	],
+	'thunderx-1n-48c': [
+		['RTE_MAX_NUMA_NODES', 1],
+		['RTE_MAX_LCORE', 48]
+	]
+}
+
+machine_args = ['-march=armv8-a+crc+crypto','-mcpu=thunderx']

diff --git a/config/arm/thunderxt88/arm64_thunderxt88_linux_gcc

b/config/arm/thunderxt88/arm64_thunderxt88_linux_gcc
new file mode 100644
index 000000000..9a622786c

--- /dev/null
+++ b/config/arm/thunderxt88/arm64_thunderxt88_linux_gcc

@@ -0,0 +1,16 @@
+[binaries]
+c = 'aarch64-linux-gnu-gcc'
+cpp = 'aarch64-linux-gnu-cpp'
+ar = 'aarch64-linux-gnu-gcc-ar'
+strip = 'aarch64-linux-gnu-strip'
+pkgconfig = 'aarch64-linux-gnu-pkg-config'
+pcap-config = ''
+
+[host_machine]
+system = 'linux'
+cpu_family = 'aarch64'
+cpu = 'armv8-a'
+endian = 'little'
+
+[properties]
+machine = 'thunderxt88'

diff --git a/config/arm/thunderxt88/meson.build

b/config/arm/thunderxt88/meson.build
new file mode 100644
index 000000000..2aa5d2cdd

--- /dev/null
+++ b/config/arm/thunderxt88/meson.build

@@ -0,0 +1,18 @@
+flags = {
+	'common': [
+		['RTE_CACHE_LINE_SIZE', 128],
+		['RTE_MAX_VFIO_GROUPS', 128],
+		['RTE_MACHINE', '"thunderx"'],
+		['RTE_USE_C11_MEM_MODEL', false]
+	],
+	'thunderxt88': [
+		['RTE_MAX_NUMA_NODES', 2],
+		['RTE_MAX_LCORE', 96]
+	],
+	'thunderxt88-1n-48c': [
+		['RTE_MAX_NUMA_NODES', 1],
+		['RTE_MAX_LCORE', 48]
+	]
+}
+
+machine_args = ['-mcpu=thunderxt88']

diff --git a/config/meson.build b/config/meson.build index

69f2aeb60..177cbd49c 100644

--- a/config/meson.build
+++ b/config/meson.build

@@ -63,7 +63,14 @@ meson.add_install_script('../buildtools/symlink-

drivers-solibs.sh',

 # set the machine type and cflags for it  if meson.is_cross_build()
-	machine = host_machine.cpu()
+	if not host_machine.cpu_family().startswith('aarch')
+		# don't change the machine config for aarch32/64 builds
+		# that config is set in the cross file to identify the soc
+		# we're building for
+		machine = host_machine.cpu()
+	else
+		machine = meson.get_cross_property('machine', 'default')
+	endif
 else
 	machine = get_option('machine')
 endif
--
2.20.1