[dpdk-dev] [PATCH v8 2/4] meson: add infra to support machine specific flags

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Thomas Monjalon <thomas at monjalon.net>
> Sent: Friday, April 12, 2019 5:07 AM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula at marvell.com>; Jerin Jacob
> Kollanukkaran <jerinj at marvell.com>
> Cc: dev at dpdk.org; jerinjacobk at gmail.com; yskoh at mellanox.com;
> bruce.richardson at intel.com
> Subject: Re: [dpdk-dev] [PATCH v8 2/4] meson: add infra to support machine
> specific flags
> 
> 10/04/2019 18:13, jerinjacobk at gmail.com:
> > From: Pavan Nikhilesh <pbhagavatula at marvell.com>
> >
> > Currently, RTE_* flags are set based on the implementer ID but there
> > might be some micro arch specific differences from the same vendor eg.
> > CACHE_LINESIZE. Add support to set micro arch specific flags.
> 
> I don't like how flags are set in config/arm/meson.build.
> It is a real mess to find which flag applies to which machine.
> Adding the flags_*_extra in the machine_args_* is adding more mess.
> 
> [...]
> >  flags_common_default = [
> >  	# Accelarate rte_memcpy. Be sure to run unit test
> (memcpy_perf_autotest)
> >  	# to determine the best threshold in code. Refer to notes in source
> > file @@ -52,12 +33,10 @@ flags_generic = [
> >  	['RTE_USE_C11_MEM_MODEL', true],
> >  	['RTE_CACHE_LINE_SIZE', 128]]
> >  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_USE_C11_MEM_MODEL', false]]
> > +	['RTE_MAX_VFIO_GROUPS', 128]]
> >  flags_dpaa = [
> >  	['RTE_MACHINE', '"dpaa"'],
> >  	['RTE_USE_C11_MEM_MODEL', true],
> > @@ -71,6 +50,27 @@ flags_dpaa2 = [
> >  	['RTE_MAX_NUMA_NODES', 1],
> >  	['RTE_MAX_LCORE', 16],
> >  	['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]]
> > +flags_default_extra = []
> > +flags_thunderx_extra = [
> > +	['RTE_MACHINE', '"thunderx"'],
> > +	['RTE_USE_C11_MEM_MODEL', false]]
> > +
> > +machine_args_generic = [
> > +	['default', ['-march=armv8-a+crc+crypto']],
> > +	['native', ['-march=native']],
> > +	['0xd03', ['-mcpu=cortex-a53']],
> > +	['0xd04', ['-mcpu=cortex-a35']],
> > +	['0xd07', ['-mcpu=cortex-a57']],
> > +	['0xd08', ['-mcpu=cortex-a72']],
> > +	['0xd09', ['-mcpu=cortex-a73']],
> > +	['0xd0a', ['-mcpu=cortex-a75']]]
> > +
> > +machine_args_cavium = [
> > +	['default', ['-march=armv8-a+crc+crypto','-mcpu=thunderx']],
> > +	['native', ['-march=native']],
> > +	['0xa1', ['-mcpu=thunderxt88'], flags_thunderx_extra],
> > +	['0xa2', ['-mcpu=thunderxt81'], flags_thunderx_extra],
> > +	['0xa3', ['-mcpu=thunderxt83'], flags_thunderx_extra]]
> 
> I think we should have a simpler model.
> We need only to know the machine name and get all the related machine
> config.
> In native compilation, machine name is guessed from implementor id and pn
> (from config/arm/armv8_machine.py). We can directly output the machine
> name from this script and leave the naming logic in this script.
> In the cross-compilation config files (config/arm/*), we can just specify the
> machine name.
> Then every machine config (machine_args and dpdk_conf) would be
> specified in some arrays based on the machine name.
> Of course, we can keep some common default values.

Thomas,

This patch was around last three months. It reached upto v8.
I think, in that last minute for RC2, We cannot take major rework on this as it needs to tested for
Other arm64 platform too. It was pulled out from RC1 because other pcap issue from meson.
Now its not fair to say to rework the meson stuff now.
I suggest to take other rework in next release.


> 
> Thoughts?
>