Re: [dpdk-dev] [PATCH v8 14/14] build: add Arm SoC meson option

[Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>]

<snip>

> >
> > >
> > > Add Arm SoC configuration to Arm meson.build and add a meson option
> > > to enable those options for native builds. This is preferable to
> > > specifying a cross file when doing aarch64 -> aarch64 builds, since
> > > the cross file specifies the toolchain as well.
> > >
> > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > ---
> > >  config/arm/arm64_armada_linux_gcc      |   6 +-
> > >  config/arm/arm64_armv8_linux_gcc       |  58 +++--------
> > >  config/arm/arm64_bluefield_linux_gcc   |   6 +-
> > >  config/arm/arm64_dpaa_linux_gcc        |   5 +-
> > >  config/arm/arm64_emag_linux_gcc        |   5 +-
> > >  config/arm/arm64_graviton2_linux_gcc   |   6 +-
> > >  config/arm/arm64_n1sdp_linux_gcc       |   6 +-
> > >  config/arm/arm64_octeontx2_linux_gcc   |   6 +-
> > >  config/arm/arm64_stingray_linux_gcc    |   6 +-
> > >  config/arm/arm64_thunderx2_linux_gcc   |   5 +-
> > >  config/arm/arm64_thunderxt88_linux_gcc |   5 +-
> > >  config/arm/meson.build                 | 132 ++++++++++++++++++++++++-
> > >  meson_options.txt                      |   2 +
> > >  13 files changed, 154 insertions(+), 94 deletions(-)
> > >

<snip>

> 
> > > +
> > > +soc = 'generic'
> > > diff --git a/config/arm/arm64_bluefield_linux_gcc
> > > b/config/arm/arm64_bluefield_linux_gcc
> > > index 6bef87fbd..7b1fae8b9 100644
> > > --- a/config/arm/arm64_bluefield_linux_gcc
> > > +++ b/config/arm/arm64_bluefield_linux_gcc
> > > @@ -13,8 +13,4 @@ cpu = 'armv8-a'
> > >  endian = 'little'
> > >
> > >  [properties]
> > > -implementer_id = '0x41'
> > > -part_number = '0xd08'
> > > -max_lcores = 16
> > > -max_numa_nodes = 1
> > > -numa = false
> > > +soc = 'bluefield'
> > > diff --git a/config/arm/arm64_dpaa_linux_gcc
> > > b/config/arm/arm64_dpaa_linux_gcc index 3458b9d7b..e52188842
> 100644
> > > --- a/config/arm/arm64_dpaa_linux_gcc
> > > +++ b/config/arm/arm64_dpaa_linux_gcc
> > > @@ -14,7 +14,4 @@ cpu = 'armv8-a'
> > >  endian = 'little'
> > >
> > >  [properties]
> > > -implementer_id = 'dpaa'
> > > -max_lcores = 16
> > > -max_numa_nodes = 1
> > > -numa = false
> > > +soc = 'dpaa'
> > > diff --git a/config/arm/arm64_emag_linux_gcc
> > > b/config/arm/arm64_emag_linux_gcc index 7cbb05510..6c24b4bca
> 100644
> > > --- a/config/arm/arm64_emag_linux_gcc
> > > +++ b/config/arm/arm64_emag_linux_gcc
> > > @@ -13,7 +13,4 @@ cpu = 'armv8-a'
> > >  endian = 'little'
> > >
> > >  [properties]
> > > -implementer_id = '0x50'
> > > -part_number = '0x0'
> > > -max_lcores = 32
> > > -max_numa_nodes = 1
> > > +soc = 'emag'
> > > diff --git a/config/arm/arm64_graviton2_linux_gcc
> > > b/config/arm/arm64_graviton2_linux_gcc
> > > index cfe239797..bae35d6be 100644
> > > --- a/config/arm/arm64_graviton2_linux_gcc
> > > +++ b/config/arm/arm64_graviton2_linux_gcc
> > > @@ -13,8 +13,4 @@ cpu = 'armv8-a'
> > >  endian = 'little'
> > >
> > >  [properties]
> > > -implementor_id = '0x41'
> > > -implementor_pn = '0xd0c'
> > > -max_lcores = 64
> > > -max_numa_nodes = 1
> > > -numa = false
> > > +soc = 'graviton2'
> > > diff --git a/config/arm/arm64_n1sdp_linux_gcc
> > > b/config/arm/arm64_n1sdp_linux_gcc
> > > index b00f2d1ef..249ff4738 100644
> > > --- a/config/arm/arm64_n1sdp_linux_gcc
> > > +++ b/config/arm/arm64_n1sdp_linux_gcc
> > > @@ -13,8 +13,4 @@ cpu = 'armv8-a'
> > >  endian = 'little'
> > >
> > >  [properties]
> > > -implementer_id = '0x41'
> > > -part_number = '0xd0c'
> > > -max_lcores = 4
> > > -max_numa_nodes = 1
> > > -numa = false
> > > +soc = 'n1sdp'
> > > diff --git a/config/arm/arm64_octeontx2_linux_gcc
> > > b/config/arm/arm64_octeontx2_linux_gcc
> > > index 593769709..063018e8f 100644
> > > --- a/config/arm/arm64_octeontx2_linux_gcc
> > > +++ b/config/arm/arm64_octeontx2_linux_gcc
> > > @@ -13,8 +13,4 @@ cpu = 'armv8-a'
> > >  endian = 'little'
> > >
> > >  [properties]
> > > -implementer_id = '0x43'
> > > -part_number = '0xb2'
> > > -max_lcores = 36
> > > -max_numa_nodes = 1
> > > -numa = false
> > > +soc = 'octeontx2'
> > > diff --git a/config/arm/arm64_stingray_linux_gcc
> > > b/config/arm/arm64_stingray_linux_gcc
> > > index 6bef87fbd..1209a8c0b 100644
> > > --- a/config/arm/arm64_stingray_linux_gcc
> > > +++ b/config/arm/arm64_stingray_linux_gcc
> > > @@ -13,8 +13,4 @@ cpu = 'armv8-a'
> > >  endian = 'little'
> > >
> > >  [properties]
> > > -implementer_id = '0x41'
> > > -part_number = '0xd08'
> > > -max_lcores = 16
> > > -max_numa_nodes = 1
> > > -numa = false
> > > +soc = 'stingray'
> > > diff --git a/config/arm/arm64_thunderx2_linux_gcc
> > > b/config/arm/arm64_thunderx2_linux_gcc
> > > index c06dcdc2b..348650712 100644
> > > --- a/config/arm/arm64_thunderx2_linux_gcc
> > > +++ b/config/arm/arm64_thunderx2_linux_gcc
> > > @@ -13,7 +13,4 @@ cpu = 'armv8-a'
> > >  endian = 'little'
> > >
> > >  [properties]
> > > -implementer_id = '0x43'
> > > -part_number = '0xaf'
> > > -max_lcores = 256
> > > -max_numa_nodes = 2
> > > +soc = 'thunderx2'
> > > diff --git a/config/arm/arm64_thunderxt88_linux_gcc
> > > b/config/arm/arm64_thunderxt88_linux_gcc
> > > index 3ba1528e4..d31d0c6d8 100644
> > > --- a/config/arm/arm64_thunderxt88_linux_gcc
> > > +++ b/config/arm/arm64_thunderxt88_linux_gcc
> > > @@ -13,7 +13,4 @@ cpu = 'armv8-a'
> > >  endian = 'little'
> > >
> > >  [properties]
> > > -implementer_id = '0x43'
> > > -part_number = '0xa1'
> > > -max_lcores = 96
> > > -max_numa_nodes = 1
> > > +soc = 'thunderxt88'
> > > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > > 52274c0c5..cd270f84b 100644
> > > --- a/config/arm/meson.build
> > > +++ b/config/arm/meson.build
> > > @@ -57,7 +57,7 @@ part_number_config_arm = {  ## Part numbers are
> > > specific to Arm implementers  # implementer specific aarch64 flags
> > > have middle priority
> > >  #     (will overwrite common flags)
> > > -# part number specific aarch64 flags have the highest priority
> > > +# part number specific aarch64 flags have higher priority
> > >  #     (will overwrite both common and implementer specific flags)
> > >  implementers = {
> > >  	'generic': {
> > > @@ -164,6 +164,102 @@ implementers = {
> > >  	}
> > >  }
> > >
> > > +# soc specific aarch64 flags have the highest priority
> > > +#     (will overwrite all other flags)
> > > +socs = {
> > Too big to maintain
> >
> 
> I don't have great alternate solutions that would work:
> 1. Possibly adding 'soc' under machine specific configuration. That would
> result in terrible code (O(N)) which would have to go through all
> implementer/part numbers to find the soc configuration.
> 2. Move the config from this dict to machine specific configuration as in 1, but
> also keep the backwards mapping of soc->(implementer, part_number). That
> way we'll have sane code which just does dict lookups (O(log N), much better).
> 
> Actually, the second option doesn't look that bad. We'll end up with extra
> mapping like this:
> 
> 'bluefield': {'implementer': '0x41', 'part_number': '0xd08'}
> 
> This would be in the soc dict and then the extra soc configuration in part
> number config.
> 
> NOTE: some socs have the same flags as their corresponsing part number flags
> (actually all of them except n1sdp). Do we want to make soc flags optional or
> do we want them to be speficied even when they won't change the config?
I think soc flags should be optional. Add the flags if they are not their in part number flags or if the part number flags need to be overwritten.

I think the solution for socs will be the same as for 'implementers'. We can come up with the syntax for 'implementers' apply the same here.

> 
> > > +	'generic': {
> > > +		'implementer': 'generic',
> > > +		'part_number': 'generic',
> > > +		'flags': []
> > > +	},
> > > +	'armada': {
> > > +		'implementer': '0x56',
> > > +		'flags': [
> > > +			['RTE_MAX_LCORE', 16],
> > > +			['RTE_MAX_NUMA_NODES', 1]
> > > +		],
> > > +		'numa': false,
> > > +		'disabled_drivers': ['bus/dpaa', 'bus/fslmc', 'common/dpaax']
> > > +	},
> > > +	'bluefield': {
> > > +		'implementer': '0x41',
> > > +		'part_number': '0xd08',
> > > +		'flags': [
> > > +			['RTE_MAX_LCORE', 16],
> > > +			['RTE_MAX_NUMA_NODES', 1]
> > > +		],
> > > +		'numa': false
> > > +	},
> > > +	'dpaa': {
> > > +		'implementer': 'dpaa',
> > > +		'flags': [
> > > +			['RTE_MAX_LCORE', 16],
> > > +			['RTE_MAX_NUMA_NODES', 1]
> > > +		],
> > > +		'numa': false
> > > +	},
> > > +	'emag': {
> > > +		'implementer': '0x50',
> > > +		'part_number': '0x0',
> > > +		'flags': [
> > > +			['RTE_MAX_LCORE', 32],
> > > +			['RTE_MAX_NUMA_NODES', 1]
> > > +		]
> > > +	},
> > > +	'graviton2': {
> > > +		'implementer': '0x41',
> > > +		'part_number': '0xd0c',
> > > +		'flags': [
> > > +			['RTE_MAX_LCORE', 64],
> > > +			['RTE_MAX_NUMA_NODES', 1]
> > > +		],
> > > +		'numa': false
> > > +	},
> > > +	'n1sdp': {
> > > +		'implementer': '0x41',
> > > +		'part_number': '0xd0c',
> > > +		'flags': [
> > > +			['RTE_MAX_LCORE', 4],
> > > +			['RTE_MAX_NUMA_NODES', 1]
> > > +		],
> > > +		'numa': false
> > > +	},
> > > +	'octeontx2': {
> > > +		'implementer': '0x43',
> > > +		'part_number': '0xb2',
> > > +		'flags': [
> > > +			['RTE_MAX_LCORE', 32],
> > > +			['RTE_MAX_NUMA_NODES', 1]
> > > +		],
> > > +		'numa': false
> > > +	},
> > > +	'stingray': {
> > > +		'implementer': '0x41',
> > > +		'part_number': '0xd08',
> > > +		'flags': [
> > > +			['RTE_MAX_LCORE', 16],
> > > +			['RTE_MAX_NUMA_NODES', 1]
> > > +		],
> > > +		'numa': false
> > > +	},
> > > +	'thunderx2': {
> > > +		'implementer': '0x43',
> > > +		'part_number': '0xaf',
> > > +		'flags': [
> > > +			['RTE_MAX_LCORE', 256],
> > > +			['RTE_MAX_NUMA_NODES', 2]
> > > +		]
> > > +	},
> > > +	'thunderxt88': {
> > > +		'implementer': '0x43',
> > > +		'part_number': '0xa1',
> > > +		'flags': [
> > > +			['RTE_MAX_LCORE', 96],
> > > +			['RTE_MAX_NUMA_NODES', 1]
> > > +		]
> > > +	}
> > > +}
> > > +
> > >  dpdk_conf.set('RTE_ARCH_ARM', 1)
> > >  dpdk_conf.set('RTE_FORCE_INTRINSICS', 1)
> > >
> > > @@ -176,11 +272,18 @@ if dpdk_conf.get('RTE_ARCH_32')  else
> > >  	# aarch64 build
> > >  	use_native_machine_args = false
> > > +	arm_soc = get_option('arm_soc')
> > > +	soc_config = {}
> > >  	if not meson.is_cross_build()
> > >  		if machine == 'generic'
> > >  			# generic build
> > > +			if arm_soc != ''
> > > +				error('Arm SoC is unsupported with generic
> > > build.')
> > > +			endif
> > >  			implementer_id = 'generic'
> > >  			part_number = 'generic'
> > > +		elif arm_soc != ''
> > > +			soc_config = socs.get(arm_soc, {'not_supported':
> > > true})
> > >  		else
> > >  			# native build
> > >  			# The script returns ['Implementer', 'Variant',
> > 'Architecture', @@
> > > -199,8 +302,27 @@ else
> > >  		endif
> > >  	else
> > >  		# cross build
> > > -		implementer_id =
> > > meson.get_cross_property('implementer_id')
> > > -		part_number = meson.get_cross_property('part_number')
> > > +		arm_soc = meson.get_cross_property('soc', '')
> > > +		if arm_soc == ''
> > > +			error('Arm SoC must be specified in the cross file.')
> > > +		endif
> > > +		soc_config = socs.get(arm_soc, {'not_supported': true})
> > > +	endif
> > > +
> > > +	soc_flags = []
> > > +	if soc_config.has_key('not_supported')
> > > +		error('SoC @0@ not supported.'.format(arm_soc))
> > > +	elif soc_config != {}
> > > +		implementer_id = soc_config['implementer']
> > > +		implementer_config = implementers[implementer_id]
> > > +		part_number = soc_config['part_number']
> > > +		soc_flags = soc_config['flags']
> > > +		if not soc_config.get('numa', true)
> > > +			has_libnuma = 0
> > > +		endif
> > > +		if soc_config.has_key('disabled_drivers')
> > > +			disabled_drivers += soc_config['disabled_drivers']
> > > +		endif
> > >  	endif
> > >
> > >  	if implementers.has_key(implementer_id)
> > > @@ -226,8 +348,8 @@ else
> > >  		      '(-Dmachine=generic) build.')
> > >  	endif
> > >
> > > -	# use default flags with implementer flags
> > > -	dpdk_flags = flags_common_default + implementer_config['flags'] +
> > > part_number_config.get('flags', [])
> > > +	# add flags in the proper order
> > > +	dpdk_flags = flags_common_default + implementer_config['flags'] +
> > > +part_number_config.get('flags', []) + soc_flags
> > >
> > >  	# apply supported machine args
> > >  	machine_args = [] # Clear previous machine args diff --git
> > > a/meson_options.txt b/meson_options.txt index e1059fb16..33b8b236c
> > > 100644
> > > --- a/meson_options.txt
> > > +++ b/meson_options.txt
> > > @@ -1,5 +1,7 @@
> > >  # Please keep these options sorted alphabetically.
> > >
> > > +option('arm_soc', type: 'string', value: '',
> > > +	description: 'Specify if you want to build for a particular Arm
> > > +SoC when building on an aarch64 machine.')
> > Kind of indicates that it is a cross build.
> 
> What do you mean by cross build in this context? It's not a meson cross build,
I meant, the sentence conveys that there is a target Arm SoC and the build machine is a different machine (though it is aarch64). May be I am reading into it too much.

> since it doesn't mention cross files, so I don't think you meant that. Aside from
> that, how does building on an aarch64 machine for aarch64 target indicate a
> cross build? I guess it says Arm SoC, so it could be a aarch64 -> armv7 cross
> build? Then I could change it to "a particular aarch64 Arm SoC".
I think this is better. We could document it better in the docs.

> 
> > How about: "Build natively for the specified Arm SoC"?
> >
> 
> I specifically didn't want to use the word native here because arm native build
> implies ['-march=native', '-mtune=native'], which won't be used in soc builds.
Agree, better to avoid the word 'native'

> Also, this wording assumes an aarch64 build machine, which is likely a safe
> assumption, but I wanted to avoid the assumption and spell it out.
> 
> > >  option('armv8_crypto_dir', type: 'string', value: '',
> > >  	description: 'path to the armv8_crypto library installation
> > > directory') option('disable_drivers', type: 'string', value: '',
> > > --
> > > 2.20.1