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 <pbhagavat...@caviumnetworks.com>
> ---
>
> 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?
> 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.py
>
> diff --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.
> 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
