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

[Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>]

Hi Herbert,

Thanks for the review, will add a default entry for generic arm.

On Mon, Jan 22, 2018 at 05:52:36AM +0000, Herbert Guan wrote:
> 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>
> > ---
<snip>
> > +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])
> > +