Re: [PATCH] build: add support for detecting march on ARM
From: Bruce Richardson <hidden>
Date: 2018-01-08 17:05:52
On Sat, Dec 30, 2017 at 10:07:54PM +0530, Pavan Nikhilesh wrote:
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 <redacted> --- The current method used for reading MIDR_EL1 form userspace might not be reliable and can be easily modified by updating config/arm/machine.py. More info on midr_el1 can be found at http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0500g/BABFEABI.html This patch depends on http://dpdk.org/dev/patchwork/patch/32410/
I had intended that patch to just be a prototype to start adding ARM support - have you considered taking that and rolling in these changes into that as a part of a set?
quoted hunk ↗ jump to hunk
config/arm/machine.py | 18 ++++++++++++++++++ config/arm/meson.build | 20 ++++++++++++++++++++ config/meson.build | 3 ++- drivers/meson.build | 2 +- examples/meson.build | 2 +- lib/meson.build | 2 +- meson.build | 2 +- 7 files changed, 44 insertions(+), 5 deletions(-) create mode 100755 config/arm/machine.pydiff --git a/config/arm/machine.py b/config/arm/machine.py new file mode 100755 index 000000000..3c6e7b6a7 --- /dev/null +++ b/config/arm/machine.py@@ -0,0 +1,18 @@ +#!/usr/bin/python +import pprint +pp = pprint + +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 250958415..f6ae69c21 100644 --- a/config/arm/meson.build +++ b/config/arm/meson.build@@ -41,3 +41,23 @@ else endif dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128) dpdk_conf.set('RTE_FORCE_INTRINSICS', 1) + +detect_vendor = find_program(join_paths(meson.current_source_dir(), + 'machine.py')) +cmd = run_command(detect_vendor.path()) +if cmd.returncode() != 0 + message('Unable to read midr_el1') +else + cmd_output = cmd.stdout().strip().split(' ') + message('midr_el1 output: \n' + 'Implementor ' + cmd_output[0] + + ' Variant ' + cmd_output[1] + ' Architecture ' + + cmd_output[2] + ' Primary Part number ' + cmd_output[3] + + ' Revision ' + cmd_output[4]) + if cmd_output[0] == '0x43' + message('Implementor : Cavium') + dpdk_conf.set('RTE_MACHINE', 'thunderx') + machine_arg = [] + machine_arg += '-march=' + 'armv8-a+crc+crypto' + machine_arg += '-mcpu=' + 'thunderx' + endif +endif
Should the call to the script and return code not be dependent on the current value of machine i.e. only if it's "native"? If the user has specified a "machine" type as part of the meson configuration parameters, you should not override it. Similarly in the cross-build case, the machine type is taken from the cross-build file itself.
quoted hunk ↗ jump to hunk
diff --git a/config/meson.build b/config/meson.build index 86e978fb1..fe8104676 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_arg = [] +machine_arg += '-march=' + machine
I was confused initially as to why this change, but now I realise it's due to the fact that for the thunderx build you need both an -march and an -mcpu flag. I think it might be better to make this change as a separate patch and rename the variable from "machine_arg" to "machine_args" to make it clearer it's an array. Alternatively, we can have separate variables for march flag and mcpu flag. [Does cpu-type need to be a configuration parameter for ARM platforms, in the non-cross-build case?] /Bruce