[dpdk-dev] [PATCH v8 04/14] build: reformat and move Arm config and comments

[Honnappa Nagarahalli]

<snip>

> > > Change formatting so that it's more consistent and readable,
> > > add/modify comments/stdout messages, move configuration options to
> > > more appropriate places and make the order consistent according to
> > > these
> > rules:
> > > 1. First list generic configuration options, then list options that may
> > >    be overwritten. List SoC-specific options last.
> > > 2. For SoC-specific options, list number of cores before the number of
> > >    NUMA nodes, to make it consistent with config/meson.build.
> > >
> > > Signed-off-by: Juraj Linkeš <juraj.linkes at pantheon.tech>
> > Few nits, otherwise, looks good.
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli at arm.com>
> >
> > > ---
> > >  config/arm/arm64_armv8_linux_gcc | 21 ++++++-
> > >  config/arm/meson.build           | 94 +++++++++++++++++++-------------
> > >  2 files changed, 77 insertions(+), 38 deletions(-)
> > >
> > > diff --git a/config/arm/arm64_armv8_linux_gcc
> > > b/config/arm/arm64_armv8_linux_gcc
> > > index 13ee8b223..04cd82ba9 100644
> > > --- a/config/arm/arm64_armv8_linux_gcc
> > > +++ b/config/arm/arm64_armv8_linux_gcc
> > > @@ -13,9 +13,16 @@ cpu = 'armv8-a'
> > >  endian = 'little'
> > >

<snip>

> > >
> > > +# implementer specific aarch64 flags, with middle priority # (will
> > Nit                                          ^^^^^^^ can be removed
> >
> > > +overwrite common flags)
> > >  flags_implementer_generic = [
> > >  	['RTE_MACHINE', '"armv8a"'],
> > > -	['RTE_MAX_LCORE', 256],
> > >  	['RTE_USE_C11_MEM_MODEL', true],
> > > -	['RTE_CACHE_LINE_SIZE', 128]]
> > > +	['RTE_CACHE_LINE_SIZE', 128],
> > > +	['RTE_MAX_LCORE', 256]
> > > +]
> >
> > <snip>
> >
> > >  flags_implementer_armada = [
> > >  	['RTE_MACHINE', '"armv8a"'],
> > >  	['RTE_CACHE_LINE_SIZE', 64],
> > > -	['RTE_MAX_NUMA_NODES', 1],
> > > -	['RTE_MAX_LCORE', 16]]
> > > +	['RTE_MAX_LCORE', 16],
> > > +	['RTE_MAX_NUMA_NODES', 1]
> > > +]
> > >
> > > +# part number specific aarch64 flags, with highest priority # (will
> > Nit                                         ^^^^^^^ can be removed
> >
> 
> I added the aarch64 specifier since we're also defining armv7 build in this file,
Agree, no need to change

> however briefly. I thought Implementer ID and part number also exist on
> armv7, is that not right? I wanted to make it explicit that these flags would
> only be used for aarch64 builds, but I guess it's implicit enough.
> 
> > > +overwrite both common and implementer specific flags)
> > >  flags_part_number_thunderx = [
> > >  	['RTE_MACHINE', '"thunderx"'],
> > > -	['RTE_USE_C11_MEM_MODEL', false]]
> > > +	['RTE_USE_C11_MEM_MODEL', false]
> > > +]
> >
> > <snip>
> >
> > >  flags_part_number_n1generic = [
> > >  	['RTE_MACHINE', '"neoverse-n1"'],
> > > -	['RTE_MAX_LCORE', 64],
> > > -	['RTE_CACHE_LINE_SIZE', 64],
> > >  	['RTE_ARM_FEATURE_ATOMICS', true],
> > >  	['RTE_USE_C11_MEM_MODEL', true],
> > > -	['RTE_MAX_MEM_MB', 1048576],
> > > -	['RTE_MAX_NUMA_NODES', 1],
> > >  	['RTE_EAL_NUMA_AWARE_HUGEPAGES', false],
> > > -	['RTE_LIBRTE_VHOST_NUMA', false]]
> > > +	['RTE_LIBRTE_VHOST_NUMA', false],
> > > +	['RTE_MAX_MEM_MB', 1048576],
> > > +	['RTE_CACHE_LINE_SIZE', 64],
> > > +	['RTE_MAX_LCORE', 64],
> > > +	['RTE_MAX_NUMA_NODES', 1]
> > > +]
> > >
> > > +# arm config (implementer 0x41) is the default config
> > I do not understand this comment. What does 'default' config mean? Are
> > you referring to 'generic' config?
> >
> 
> I am using the dictionary definition - use the config when no other alternative
> is found. We're using part_number_config_arm in four places, implementers
> generic, 0x41, 0x56, dpaa. For 0x41, it's the actual config, the other
> implementers default to it since we don't have specific config for them.
Ok

> 
> > >  part_number_config_arm = [
> > >  	['generic', ['-march=armv8-a+crc', '-moutline-atomics']],
> > >  	['native', ['-march=native']],
> > > @@ -96,8 +114,8 @@ part_number_config_arm = [
> > >  	['0xd09', ['-mcpu=cortex-a73']],
> > >  	['0xd0a', ['-mcpu=cortex-a75']],
> > >  	['0xd0b', ['-mcpu=cortex-a76']],
> > > -	['0xd0c', ['-march=armv8.2-a+crypto', '-mcpu=neoverse-n1'],
> > > flags_part_number_n1generic]]
> > > -
> > > +	['0xd0c', ['-march=armv8.2-a+crypto', '-mcpu=neoverse-n1'],
> > > +flags_part_number_n1generic] ]
> > >  part_number_config_cavium = [
> > >  	['generic', ['-march=armv8-a+crc+crypto','-mcpu=thunderx']],
> > >  	['native', ['-march=native']],
> > > @@ -105,13 +123,14 @@ part_number_config_cavium = [
> > >  	['0xa2', ['-mcpu=thunderxt81'], flags_part_number_thunderx],
> > >  	['0xa3', ['-mcpu=thunderxt83'], flags_part_number_thunderx],
> > >  	['0xaf', ['-march=armv8.1-a+crc+crypto','-mcpu=thunderx2t99'],
> > > flags_part_number_thunderx2],
> > > -	['0xb2', ['-march=armv8.2-a+crc+crypto+lse','-mcpu=octeontx2'],
> > > flags_part_number_octeontx2]]
> > > -
> > > +	['0xb2', ['-march=armv8.2-a+crc+crypto+lse','-mcpu=octeontx2'],
> > > +flags_part_number_octeontx2] ]
> > >  part_number_config_emag = [
> > >  	['generic', ['-march=armv8-a+crc+crypto', '-mtune=emag']],
> > > -	['native', ['-march=native']]]
> > > +	['native', ['-march=native']]
> > > +]
> > >
> > > -## Arm implementer ID (ARM DDI 0487C.a, Section G7.2.106, Page
> > > G7-5321)
> > > +## Arm implementer ID (MIDR in Arm Architecture Reference Manual)
> > >  implementer_generic = ['Generic armv8', flags_implementer_generic,
> > > part_number_config_arm]
> > >  implementer_0x41 = ['Arm', flags_implementer_arm,
> > > part_number_config_arm]
> > >  implementer_0x43 = ['Cavium', flags_implementer_cavium,
> > > part_number_config_cavium] @@ -123,21 +142,21 @@
> > > dpdk_conf.set('RTE_ARCH_ARM', 1)
> > > dpdk_conf.set('RTE_FORCE_INTRINSICS',
> > > 1)
> > >
> > >  if dpdk_conf.get('RTE_ARCH_32')
> > > +	# armv7 build
> > >  	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
> > >  	dpdk_conf.set('RTE_ARCH_ARMv7', 1)
> > >  	# the minimum architecture supported, armv7-a, needs the following,
> > > -	# mk/machine/armv7a/rte.vars.mk sets it too
> > >  	machine_args += '-mfpu=neon'
> > >  else
> > > -	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128)
> > > -	dpdk_conf.set('RTE_ARCH_ARM64', 1)
> > > -
> > > +	# aarch64 build
> > >  	implementer_id = 'generic'
> > >  	machine_args = [] # Clear previous machine args
> > >  	if machine == 'generic' and not meson.is_cross_build()
> > > +		# generic build
> > >  		implementer_config = implementer_generic
> > >  		part_number = 'generic'
> > >  	elif not meson.is_cross_build()
> > > +		# native build
> > >  		# The script returns ['Implementer', 'Variant', 'Architecture',
> > >  		# 'Primary Part number', 'Revision']
> > >  		detect_vendor = find_program(join_paths( @@ -158,6 +177,7
> > @@ else
> > >  			part_number = 'native'
> > >  		endif
> > >  	else
> > > +		# cross build
> > >  		implementer_id =
> > > meson.get_cross_property('implementer_id', 'generic')
> > >  		part_number = meson.get_cross_property('part_number',
> > > 'generic')
> > >  		implementer_config = get_variable('implementer_' +
> > > implementer_id) @@ -194,7 +214,7 @@ else
> > >  		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) != '')
> > > --
> > > 2.20.1