[dpdk-dev] build: add support for detecting march on ARM
Checks
Commit Message
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>
---
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/
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
--
2.15.1
Comments
-----Original Message-----
> Date: Sat, 30 Dec 2017 22:07:54 +0530
> From: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> To: bruce.richardson@intel.com, bluca@debian.org,
> harry.van.haaren@intel.com, jerin.jacob@caviumnetworks.com
> Cc: dev@dpdk.org, Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> Subject: [dpdk-dev] [PATCH] build: add support for detecting march on ARM
> X-Mailer: git-send-email 2.14.1
>
> 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>
> ---
>
> 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/
>
> 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')
Instead of this "unable to read midr_el1", We could fallback to
"message('Implementer : Generic arm64')" message
Some thoughts,
1) From the Linux documentation[1] this sysfs entries are available
from the kernels > June 2016
[1]
What: /sys/devices/system/cpu/cpuX/regs/
/sys/devices/system/cpu/cpuX/regs/identification/
/sys/devices/system/cpu/cpuX/regs/identification/midr_el1
/sys/devices/system/cpu/cpuX/regs/identification/revidr_el1
Date: June 2016
Contact: Linux ARM Kernel Mailing list <linux-arm-kernel@lists.infradead.org>
Description: AArch64 CPU registers 'identification' directory exposes the CPU ID registers or
identifying model and revision of the CPU.
2) This scheme is not available in Freebsd.
3) I guess, we anyway need a meta file to store this information for cross compilation case.
If so,
1) Can we introduce something similar to T= for cross compilation case and
use the same scheme to override for native compilation flags if required.
2) If above scheme is not chosen and
a) if /sys/devices/system/cpu/cpuX/regs/identification/midr_el1 avilable then derive
machine_arg based on that
b) if /sys/devices/system/cpu/cpuX/regs/identification/midr_el1 not available then fallback to generic arm64 compilation.
Thoughts?
> +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'
1) Instead of changing the code for each Vendors and the multiple
variants, Can we put some framework to just define this information some
where?
2) Please add the another mcpu variables for the Cavium produces like
81xx and 83xx etc
> + endif
> +endif
> 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
>
> # use pthreads
> add_project_link_arguments('-pthread', language: 'c')
> diff --git a/drivers/meson.build b/drivers/meson.build
> index 9b5039847..f5009aa2e 100644
> --- a/drivers/meson.build
> +++ b/drivers/meson.build
> @@ -22,7 +22,7 @@ foreach class:driver_classes
> version = 1
> sources = []
> objs = []
> - cflags = [machine_arg]
> + cflags = machine_arg
> includes = [include_directories(drv_path)]
> # set up internal deps. Drivers can append/override as necessary
> deps = std_deps
> diff --git a/examples/meson.build b/examples/meson.build
> index 0abed7169..88df650cf 100644
> --- a/examples/meson.build
> +++ b/examples/meson.build
> @@ -9,7 +9,7 @@ endif
> foreach example: get_option('examples').split(',')
> name = example
> sources = []
> - cflags = [machine_arg]
> + cflags = machine_arg
> ext_deps = []
> includes = [include_directories(example)]
> deps = ['eal', 'mempool', 'net', 'mbuf', 'ethdev', 'cmdline']
> diff --git a/lib/meson.build b/lib/meson.build
> index 0c94d74b9..3c98efa70 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -31,7 +31,7 @@ foreach l:libraries
> sources = []
> headers = []
> includes = []
> - cflags = [machine_arg]
> + cflags = machine_arg
> objs = [] # other object files to link against, used e.g. for
> # instruction-set optimized versions of code
>
> diff --git a/meson.build b/meson.build
> index 3dce8579e..493bcc313 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -66,5 +66,5 @@ pkg.generate(name: meson.project_name(),
> ['-Wl,-Bdynamic'] + dpdk_extra_ldflags,
> description: 'The Data Plane Development Kit (DPDK)',
> subdirs: [get_option('include_subdir_arch'), '.'],
> - extra_cflags: ['-include "rte_config.h"', machine_arg]
> + extra_cflags: ['-include "rte_config.h"'] + machine_arg
> )
> --
> 2.15.1
>
On Mon, Jan 08, 2018 at 01:43:07PM +0530, Jerin Jacob wrote:
> -----Original Message-----
> > Date: Sat, 30 Dec 2017 22:07:54 +0530
> > From: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > To: bruce.richardson@intel.com, bluca@debian.org,
> > harry.van.haaren@intel.com, jerin.jacob@caviumnetworks.com
> > Cc: dev@dpdk.org, Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > Subject: [dpdk-dev] [PATCH] build: add support for detecting march on ARM
> > X-Mailer: git-send-email 2.14.1
> >
> > 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>
> > +cmd = run_command(detect_vendor.path())
> > +if cmd.returncode() != 0
> > + message('Unable to read midr_el1')
>
> Instead of this "unable to read midr_el1", We could fallback to
> "message('Implementer : Generic arm64')" message
>
> Some thoughts,
> 1) From the Linux documentation[1] this sysfs entries are available
> from the kernels > June 2016
>
> [1]
> What: /sys/devices/system/cpu/cpuX/regs/
> /sys/devices/system/cpu/cpuX/regs/identification/
> /sys/devices/system/cpu/cpuX/regs/identification/midr_el1
> /sys/devices/system/cpu/cpuX/regs/identification/revidr_el1
> Date: June 2016
> Contact: Linux ARM Kernel Mailing list <linux-arm-kernel@lists.infradead.org>
> Description: AArch64 CPU registers 'identification' directory exposes the CPU ID registers or
> identifying model and revision of the CPU.
>
> 2) This scheme is not available in Freebsd.
> 3) I guess, we anyway need a meta file to store this information for cross compilation case.
>
> If so,
> 1) Can we introduce something similar to T= for cross compilation case and
> use the same scheme to override for native compilation flags if required.
> 2) If above scheme is not chosen and
> a) if /sys/devices/system/cpu/cpuX/regs/identification/midr_el1 avilable then derive
> machine_arg based on that
> b) if /sys/devices/system/cpu/cpuX/regs/identification/midr_el1 not available then fallback to generic arm64 compilation.
>
> Thoughts?
>
[1&2] Agreed, when midr_el1 file is unavailable should continue with generic arm32/64.
This would cover freebsd too and if we need we can use the meta stored in cross
compilation case for native compilation.
It would be good if we start agreeing in directory structure where cross-compile
meta files are stored so that we can start adding and experiment with them.
The meta files would have info as mentioned in
http://dpdk.org/dev/patchwork/patch/32410/
Thanks,
Pavan.
>
> > +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'
>
> 1) Instead of changing the code for each Vendors and the multiple
> variants, Can we put some framework to just define this information some
> where?
> 2) Please add the another mcpu variables for the Cavium produces like
> 81xx and 83xx etc
>
>
> > + endif
> > +endif
> > 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
> >
> > # use pthreads
> > add_project_link_arguments('-pthread', language: 'c')
> > diff --git a/drivers/meson.build b/drivers/meson.build
> > index 9b5039847..f5009aa2e 100644
> > --- a/drivers/meson.build
> > +++ b/drivers/meson.build
> > @@ -22,7 +22,7 @@ foreach class:driver_classes
> > version = 1
> > sources = []
> > objs = []
> > - cflags = [machine_arg]
> > + cflags = machine_arg
> > includes = [include_directories(drv_path)]
> > # set up internal deps. Drivers can append/override as necessary
> > deps = std_deps
> > diff --git a/examples/meson.build b/examples/meson.build
> > index 0abed7169..88df650cf 100644
> > --- a/examples/meson.build
> > +++ b/examples/meson.build
> > @@ -9,7 +9,7 @@ endif
> > foreach example: get_option('examples').split(',')
> > name = example
> > sources = []
> > - cflags = [machine_arg]
> > + cflags = machine_arg
> > ext_deps = []
> > includes = [include_directories(example)]
> > deps = ['eal', 'mempool', 'net', 'mbuf', 'ethdev', 'cmdline']
> > diff --git a/lib/meson.build b/lib/meson.build
> > index 0c94d74b9..3c98efa70 100644
> > --- a/lib/meson.build
> > +++ b/lib/meson.build
> > @@ -31,7 +31,7 @@ foreach l:libraries
> > sources = []
> > headers = []
> > includes = []
> > - cflags = [machine_arg]
> > + cflags = machine_arg
> > objs = [] # other object files to link against, used e.g. for
> > # instruction-set optimized versions of code
> >
> > diff --git a/meson.build b/meson.build
> > index 3dce8579e..493bcc313 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -66,5 +66,5 @@ pkg.generate(name: meson.project_name(),
> > ['-Wl,-Bdynamic'] + dpdk_extra_ldflags,
> > description: 'The Data Plane Development Kit (DPDK)',
> > subdirs: [get_option('include_subdir_arch'), '.'],
> > - extra_cflags: ['-include "rte_config.h"', machine_arg]
> > + extra_cflags: ['-include "rte_config.h"'] + machine_arg
> > )
> > --
> > 2.15.1
> >
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 <pbhagavatula@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
On Mon, Jan 08, 2018 at 05:05:47PM +0000, Bruce Richardson wrote:
> 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 <pbhagavatula@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?
Agreed, I will merge the patches as a patchset in v2.
>
> > 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.
>
Yes, only in native build scenario the script and result should be used I will
modify that in v2.
> > 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.
I was thinking of having a dpdk_machine_args in base meson.build parallel to
dpdk_extra_ldflags and friends.
> 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?]
>
Initially while experimenting with meson I did try out splitting the march and
mcpu flags but that resulted in extra overhead i.e. filtering out mcpu when
it's unused in case of x86 builds.
In case of x86 the mcpu flag is deprecated where as in arm it provides extra
hint to the compiler to do silicon specific optimizations as the implementation
of core is vendor specific. In case of gcc7[1] the mcpu can be furthur tuned to
be core specific by basing it around the primary part number
(thunderxt81(0xA2), thunderxt83(0xA3), thunderxt88(0xA1)) detected via reading
midr_el1.
[1] https://gcc.gnu.org/gcc-7/changes.html
> /Bruce
>
Thanks,
Pavan.
On Wed, Jan 10, 2018 at 01:51:30PM +0530, Pavan Nikhilesh wrote:
> On Mon, Jan 08, 2018 at 05:05:47PM +0000, Bruce Richardson wrote:
> > 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 <pbhagavatula@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?
>
> Agreed, I will merge the patches as a patchset in v2.
>
> >
> > > 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.
> >
>
> Yes, only in native build scenario the script and result should be used I will
> modify that in v2.
>
> > > 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.
>
> I was thinking of having a dpdk_machine_args in base meson.build parallel to
> dpdk_extra_ldflags and friends.
>
> > 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?]
> >
>
> Initially while experimenting with meson I did try out splitting the march and
> mcpu flags but that resulted in extra overhead i.e. filtering out mcpu when
> it's unused in case of x86 builds.
>
> In case of x86 the mcpu flag is deprecated where as in arm it provides extra
> hint to the compiler to do silicon specific optimizations as the implementation
> of core is vendor specific. In case of gcc7[1] the mcpu can be furthur tuned to
> be core specific by basing it around the primary part number
> (thunderxt81(0xA2), thunderxt83(0xA3), thunderxt88(0xA1)) detected via reading
> midr_el1.
>
> [1] https://gcc.gnu.org/gcc-7/changes.html
>
Ok, go with your original idea then of making machine_args an array, it
seems to make most sense, and also allows further expandability in
future too in case some architectures also want to use a -mtune option.
[Just add an "s" to the name! :-)]
Thanks,
/Bruce
new file mode 100755
@@ -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))
@@ -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
@@ -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
# use pthreads
add_project_link_arguments('-pthread', language: 'c')
@@ -22,7 +22,7 @@ foreach class:driver_classes
version = 1
sources = []
objs = []
- cflags = [machine_arg]
+ cflags = machine_arg
includes = [include_directories(drv_path)]
# set up internal deps. Drivers can append/override as necessary
deps = std_deps
@@ -9,7 +9,7 @@ endif
foreach example: get_option('examples').split(',')
name = example
sources = []
- cflags = [machine_arg]
+ cflags = machine_arg
ext_deps = []
includes = [include_directories(example)]
deps = ['eal', 'mempool', 'net', 'mbuf', 'ethdev', 'cmdline']
@@ -31,7 +31,7 @@ foreach l:libraries
sources = []
headers = []
includes = []
- cflags = [machine_arg]
+ cflags = machine_arg
objs = [] # other object files to link against, used e.g. for
# instruction-set optimized versions of code
@@ -66,5 +66,5 @@ pkg.generate(name: meson.project_name(),
['-Wl,-Bdynamic'] + dpdk_extra_ldflags,
description: 'The Data Plane Development Kit (DPDK)',
subdirs: [get_option('include_subdir_arch'), '.'],
- extra_cflags: ['-include "rte_config.h"', machine_arg]
+ extra_cflags: ['-include "rte_config.h"'] + machine_arg
)