Re: [PATCH v4 2/2] build: add support for detecting march on ARM

[Herbert Guan <Herbert.Guan@arm.com>]

Hi, Pavan

Please see my notes inline.

Best regards,
Herbert

> -----Original Message-----
> From: Pavan Nikhilesh [mailto:pbhagavatula@caviumnetworks.com]
> Sent: Saturday, January 20, 2018 2:24
> To: jerin.jacob@caviumnetworks.com; bruce.richardson@intel.com;
> harry.van.haaren@intel.com; Herbert Guan <Herbert.Guan@arm.com>;
> hemant.agrawal@nxp.com
> Cc: dev@dpdk.org; Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> Subject: [dpdk-dev] [PATCH v4 2/2] build: add support for detecting march
> on ARM
> 
> Added support for detecting march and mcpu by reading midr_el1 register.
> The implementer, primary part number values read can be used to figure out
> the underlying arm cpu.
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> ---
>  app/test-pmd/meson.build    |  2 +-
>  config/arm/armv8_machine.py | 18 +++++++++++
>  config/arm/meson.build      | 76
> ++++++++++++++++++++++++++++++++++++++++-----
>  config/meson.build          | 19 ++++++------
>  drivers/meson.build         |  2 +-
>  examples/meson.build        |  2 +-
>  lib/meson.build             |  2 +-
>  meson.build                 |  2 +-
>  test/test/meson.build       |  2 +-
>  9 files changed, 102 insertions(+), 23 deletions(-)  create mode 100755
> config/arm/armv8_machine.py
> 
> diff --git a/app/test-pmd/meson.build b/app/test-pmd/meson.build index
> e819677a5..2a3f0ba1f 100644
> --- a/app/test-pmd/meson.build
> +++ b/app/test-pmd/meson.build
> @@ -45,7 +45,7 @@ endif
> 
>  executable('dpdk-testpmd',
>  	sources,
> -	c_args: machine_arg,
> +	c_args: machine_args,
>  	link_whole: link_libs,
>  	dependencies: dep_objs,
>  	install_rpath: join_paths(get_option('prefix'), driver_install_path),
> diff --git a/config/arm/armv8_machine.py b/config/arm/armv8_machine.py
> new file mode 100755 index 000000000..404866d2f
> --- /dev/null
> +++ b/config/arm/armv8_machine.py
> @@ -0,0 +1,18 @@
> +#!/usr/bin/python
> +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2017 Cavium, Inc
> +
> +ident = []
> +fname = '/sys/devices/system/cpu/cpu0/regs/identification/midr_el1'
> +with open(fname) as f:
> +    content = f.read()
> +
> +midr_el1 = (int(content.rstrip('\n'), 16))
> +
> +ident.append(hex((midr_el1 >> 24) & 0xFF))  # Implementer
> +ident.append(hex((midr_el1 >> 20) & 0xF))   # Variant
> +ident.append(hex((midr_el1 >> 16) & 0XF))   # Architecture
> +ident.append(hex((midr_el1 >> 4) & 0xFFF))  # Primary Part number
> +ident.append(hex(midr_el1 & 0xF))           # Revision
> +
> +print(' '.join(ident))
> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> f05de4c2c..1bed82236 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -5,28 +5,88 @@
>  # for checking defines we need to use the correct compiler flags  march_opt
> = '-march=@0@'.format(machine)
> 
> +machine_args_cavium = [
> +	['default', ['-march=armv8-a+crc+crypto','-mcpu=thunderx']],
> +	['0xa1', ['-mcpu=thunderxt88']],
> +	['0xa2', ['-mcpu=thunderxt81']],
> +	['0xa3', ['-mcpu=thunderxt83']]]
> +
> +flags_cavium = [
> +	['RTE_MACHINE', '"thunderx"'],
> +	['RTE_CACHE_LINE_SIZE', 128],
> +	['RTE_MAX_NUMA_NODES', 2],
> +	['RTE_MAX_LCORE', 96],
> +	['RTE_MAX_VFIO_GROUPS', 128],
> +	['RTE_RING_USE_C11_MEM_MODEL', false]]
> +
> +impl_0x43 = ['Cavium', flags_cavium, machine_args_cavium]

There're only Cavimu args/flags defined, so other arm/arm64 platforms will fail at detecting.  Can you add one entry for default?

> +
> +dpdk_conf.set_quoted('RTE_TOOLCHAIN', 'gcc')
> +dpdk_conf.set('RTE_TOOLCHAIN_GCC', 1)
>  dpdk_conf.set('RTE_FORCE_INTRINSICS', 1) -if cc.sizeof('void *') == 8
> -	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128)
> -	dpdk_conf.set('RTE_ARCH_ARM64', 1)
> -	dpdk_conf.set('RTE_ARCH_64', 1)
> -else
> +
> +if cc.sizeof('void *') != 8
>  	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
>  	dpdk_conf.set('RTE_ARCH_ARM', 1)
>  	dpdk_conf.set('RTE_ARCH_ARMv7', 1)
> +else
> +	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128)
> +	dpdk_conf.set('RTE_ARCH_ARM64', 1)
> +	dpdk_conf.set('RTE_ARCH_64', 1)
> +
> +	if not meson.is_cross_build()
> +		# The script returns ['Implementor', '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
> +			message('Using default armv8 config')
> +		else
> +			machine_args = [] # Clear previous machine args
> +			cmd_output = cmd.stdout().strip().split(' ')
> +			machine = get_variable('impl_' + cmd_output[0])

Script will fail for non-cavium Arm platforms.  We need to check if cmd_output[0] is a known value in a list, otherwise should go to default entry.

> +			message('Implementor : ' + machine[0])
> +
> +			foreach flag: machine[1]
> +				dpdk_conf.set(flag[0], flag[1])
> +			endforeach
> +
> +			# Primary part number based mcpu flags are
> supported
> +			# for gcc versions > 7
> +			if cc.version().version_compare('<7.0')
> +				foreach marg: machine[2]
> +					if marg[0] == 'default'
> +						foreach f: marg[1]
> +							machine_args += f
> +						endforeach
> +					endif
> +				endforeach
> +			else
> +				foreach marg: machine[2]
> +					if marg[0] == cmd_output[3]
> +						foreach f: marg[1]
> +							machine_args += f
> +						endforeach
> +					endif
> +				endforeach
> +			endif
> +		endif
> +	endif
>  endif
> +message(machine_args)
> 
> -if cc.get_define('__ARM_NEON', args: march_opt) != ''
> +if cc.get_define('__ARM_NEON', args: machine_args) != ''
>  	dpdk_conf.set('RTE_MACHINE_CPUFLAG_NEON', 1)
>  	compile_time_cpuflags += ['RTE_CPUFLAG_NEON']  endif
> 
> -if cc.get_define('__ARM_FEATURE_CRC32', args: march_opt) != ''
> +if cc.get_define('__ARM_FEATURE_CRC32', args: machine_args) != ''
>  	dpdk_conf.set('RTE_MACHINE_CPUFLAG_CRC32', 1)
>  	compile_time_cpuflags += ['RTE_CPUFLAG_CRC32']  endif
> 
> -if cc.get_define('__ARM_FEATURE_CRYPTO', args: march_opt) != ''
> +if cc.get_define('__ARM_FEATURE_CRYPTO', args: machine_args) != ''
>  	dpdk_conf.set('RTE_MACHINE_CPUFLAG_AES', 1)
>  	dpdk_conf.set('RTE_MACHINE_CPUFLAG_PMULL', 1)
>  	dpdk_conf.set('RTE_MACHINE_CPUFLAG_SHA1', 1) diff --git
> a/config/meson.build b/config/meson.build index fa55c53a5..f8c67578d
> 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -8,7 +8,8 @@ else
>  	machine = get_option('machine')
>  endif
>  dpdk_conf.set('RTE_MACHINE', machine)
> -machine_arg = '-march=' + machine
> +machine_args = []
> +machine_args += '-march=' + machine
> 
>  # use pthreads
>  add_project_link_arguments('-pthread', language: 'c') @@ -53,6 +54,14 @@
> foreach arg: warning_flags
>  	endif
>  endforeach
> 
> +# set other values pulled from the build options
> +dpdk_conf.set('RTE_MAX_LCORE', get_option('max_lcores'))
> +dpdk_conf.set('RTE_MAX_NUMA_NODES',
> get_option('max_numa_nodes'))
> +dpdk_conf.set('RTE_LIBEAL_USE_HPET', get_option('use_hpet'))
> +dpdk_conf.set('RTE_EAL_ALLOW_INV_SOCKET_ID',
> +get_option('allow_invalid_socket_id'))
> +# values which have defaults which may be overridden
> +dpdk_conf.set('RTE_MAX_VFIO_GROUPS', 64)
> +
>  compile_time_cpuflags = []
>  if host_machine.cpu_family().startswith('x86')
>  	arch_subdir = 'x86'
> @@ -65,12 +74,4 @@ dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS',
> ','.join(compile_time_cpuflags))  # set the install path for the drivers
> dpdk_conf.set_quoted('RTE_EAL_PMD_PATH', eal_pmd_path)
> 
> -# set other values pulled from the build options -
> dpdk_conf.set('RTE_MAX_LCORE', get_option('max_lcores')) -
> dpdk_conf.set('RTE_MAX_NUMA_NODES', get_option('max_numa_nodes'))
> -dpdk_conf.set('RTE_LIBEAL_USE_HPET', get_option('use_hpet')) -
> dpdk_conf.set('RTE_EAL_ALLOW_INV_SOCKET_ID',
> get_option('allow_invalid_socket_id'))
> -# values which have defaults which may be overridden -
> dpdk_conf.set('RTE_MAX_VFIO_GROUPS', 64)
> -
>  install_headers('rte_config.h', subdir: get_option('include_subdir_arch'))
> diff --git a/drivers/meson.build b/drivers/meson.build index
> 9b5039847..1d6430bfe 100644
> --- a/drivers/meson.build
> +++ b/drivers/meson.build
> @@ -22,7 +22,7 @@ foreach class:driver_classes
>  		version = 1
>  		sources = []
>  		objs = []
> -		cflags = [machine_arg]
> +		cflags = machine_args
>  		includes = [include_directories(drv_path)]
>  		# set up internal deps. Drivers can append/override as
> necessary
>  		deps = std_deps
> diff --git a/examples/meson.build b/examples/meson.build index
> 0abed7169..b3f997242 100644
> --- a/examples/meson.build
> +++ b/examples/meson.build
> @@ -9,7 +9,7 @@ endif
>  foreach example: get_option('examples').split(',')
>  	name = example
>  	sources = []
> -	cflags = [machine_arg]
> +	cflags = machine_args
>  	ext_deps = []
>  	includes = [include_directories(example)]
>  	deps = ['eal', 'mempool', 'net', 'mbuf', 'ethdev', 'cmdline'] diff --git
> a/lib/meson.build b/lib/meson.build index 0c94d74b9..b9ea6f61b 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -31,7 +31,7 @@ foreach l:libraries
>  	sources = []
>  	headers = []
>  	includes = []
> -	cflags = [machine_arg]
> +	cflags = machine_args
>  	objs = [] # other object files to link against, used e.g. for
>  	          # instruction-set optimized versions of code
> 
> diff --git a/meson.build b/meson.build
> index 3dce8579e..326fbc7d1 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -66,5 +66,5 @@ pkg.generate(name: meson.project_name(),
>  			['-Wl,-Bdynamic'] + dpdk_extra_ldflags,
>  	description: 'The Data Plane Development Kit (DPDK)',
>  	subdirs: [get_option('include_subdir_arch'), '.'],
> -	extra_cflags: ['-include "rte_config.h"', machine_arg]
> +	extra_cflags: ['-include "rte_config.h"'] + machine_args
>  )
> diff --git a/test/test/meson.build b/test/test/meson.build index
> 1863c603c..873efbc07 100644
> --- a/test/test/meson.build
> +++ b/test/test/meson.build
> @@ -204,7 +204,7 @@ if get_option('tests')
>  		test_sources,
>  		link_whole: link_libs,
>  		dependencies: test_dep_objs,
> -		c_args: machine_arg,
> +		c_args: machine_args,
>  		install_rpath: driver_install_path,
>  		install: false)
> 
> --
> 2.15.1