[RFC,v1] build: add platform meson option
Checks
Commit Message
The current meson option 'machine' should only specify the ISA, which is
not sufficient for Arm, where setting ISA implies other setting as well.
Add a new meson option, 'platform', which differentiates the type of the
build (native/generic) and sets machine accordingly, unless the user
chooses to override it.
Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
---
config/arm/meson.build | 2 +-
config/meson.build | 14 +++++++++++++-
meson_options.txt | 6 ++++--
3 files changed, 18 insertions(+), 4 deletions(-)
Comments
On Thu, Nov 26, 2020 at 04:47:29PM +0100, Juraj Linkeš wrote:
> The current meson option 'machine' should only specify the ISA, which is
> not sufficient for Arm, where setting ISA implies other setting as well.
> Add a new meson option, 'platform', which differentiates the type of the
> build (native/generic) and sets machine accordingly, unless the user
> chooses to override it.
>
> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> ---
> config/arm/meson.build | 2 +-
> config/meson.build | 14 +++++++++++++-
> meson_options.txt | 6 ++++--
> 3 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/config/arm/meson.build b/config/arm/meson.build
> index 42b4e43c7..ac680956f 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -6,7 +6,7 @@
> march_opt = '-march=@0@'.format(machine)
>
> arm_force_native_march = false
> -arm_force_default_march = (machine == 'default')
> +arm_force_default_march = (platform == 'generic')
>
> flags_common_default = [
> # Accelarate rte_memcpy. Be sure to run unit test (memcpy_perf_autotest)
> diff --git a/config/meson.build b/config/meson.build
> index c02802c18..41d32e63e 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -63,6 +63,8 @@ if not is_windows
> pmd_subdir_opt)
> endif
>
> +platform = get_option('platform')
> +
> # set the machine type and cflags for it
> if meson.is_cross_build()
> machine = host_machine.cpu()
> @@ -70,13 +72,23 @@ else
> machine = get_option('machine')
> endif
>
> +if platform == 'native'
> + if machine == 'auto'
> + machine = 'native'
> + endif
> +elif platform == 'generic'
> + if machine == 'auto'
> + machine = 'default'
> + endif
> +endif
> +
> +if machine == 'default'
> # machine type 'default' is special, it defaults to the per arch agreed common
> # minimal baseline needed for DPDK.
> # That might not be the most optimized, but the most portable version while
> # still being able to support the CPU features required for DPDK.
> # This can be bumped up by the DPDK project, but it can never be an
> # invariant like 'native'
> -if machine == 'default'
> if host_machine.cpu_family().startswith('x86')
> # matches the old pre-meson build systems default
> machine = 'corei7'
> diff --git a/meson_options.txt b/meson_options.txt
> index e384e6dbb..1a5e47fd3 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -20,14 +20,16 @@ option('kernel_dir', type: 'string', value: '',
> description: 'Path to the kernel for building kernel modules. Headers must be in $kernel_dir/build. Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
> option('lib_musdk_dir', type: 'string', value: '',
> description: 'path to the MUSDK library installation directory')
> -option('machine', type: 'string', value: 'native',
> - description: 'set the target machine type')
> +option('machine', type: 'string', value: 'auto',
> + description: 'set the target machine type/ISA')
> option('max_ethports', type: 'integer', value: 32,
> description: 'maximum number of Ethernet devices')
> option('max_lcores', type: 'integer', value: 128,
> description: 'maximum number of cores/threads supported by EAL')
> option('max_numa_nodes', type: 'integer', value: 4,
> description: 'maximum number of NUMA nodes supported by EAL')
> +option('platform', type: 'string', value: 'generic',
> + description: 'Platform to build for, either "native" or "generic".')
As well as this short description option, I think we need more
comprehensive coverage of this option in the docs. Presumably for ARM
systems this will have other options for various SOC's rather than just
generic/native?
/Bruce
> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Thursday, November 26, 2020 5:03 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: thomas@monjalon.net; Honnappa.Nagarahalli@arm.com; dev@dpdk.org
> Subject: Re: [RFC PATCH v1] build: add platform meson option
>
> On Thu, Nov 26, 2020 at 04:47:29PM +0100, Juraj Linkeš wrote:
> > The current meson option 'machine' should only specify the ISA, which
> > is not sufficient for Arm, where setting ISA implies other setting as well.
> > Add a new meson option, 'platform', which differentiates the type of
> > the build (native/generic) and sets machine accordingly, unless the
> > user chooses to override it.
> >
> > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > ---
> > config/arm/meson.build | 2 +-
> > config/meson.build | 14 +++++++++++++-
> > meson_options.txt | 6 ++++--
> > 3 files changed, 18 insertions(+), 4 deletions(-)
> >
> > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > 42b4e43c7..ac680956f 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -6,7 +6,7 @@
> > march_opt = '-march=@0@'.format(machine)
> >
> > arm_force_native_march = false
> > -arm_force_default_march = (machine == 'default')
> > +arm_force_default_march = (platform == 'generic')
> >
> > flags_common_default = [
> > # Accelarate rte_memcpy. Be sure to run unit test
> > (memcpy_perf_autotest) diff --git a/config/meson.build
> > b/config/meson.build index c02802c18..41d32e63e 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -63,6 +63,8 @@ if not is_windows
> > pmd_subdir_opt)
> > endif
> >
> > +platform = get_option('platform')
> > +
> > # set the machine type and cflags for it if meson.is_cross_build()
> > machine = host_machine.cpu()
> > @@ -70,13 +72,23 @@ else
> > machine = get_option('machine')
> > endif
> >
> > +if platform == 'native'
> > + if machine == 'auto'
> > + machine = 'native'
> > + endif
> > +elif platform == 'generic'
> > + if machine == 'auto'
> > + machine = 'default'
> > + endif
> > +endif
> > +
> > +if machine == 'default'
> > # machine type 'default' is special, it defaults to the per arch
> > agreed common # minimal baseline needed for DPDK.
> > # That might not be the most optimized, but the most portable version
> > while # still being able to support the CPU features required for DPDK.
> > # This can be bumped up by the DPDK project, but it can never be an
> > # invariant like 'native'
> > -if machine == 'default'
> > if host_machine.cpu_family().startswith('x86')
> > # matches the old pre-meson build systems default
> > machine = 'corei7'
> > diff --git a/meson_options.txt b/meson_options.txt index
> > e384e6dbb..1a5e47fd3 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -20,14 +20,16 @@ option('kernel_dir', type: 'string', value: '',
> > description: 'Path to the kernel for building kernel modules.
> > Headers must be in $kernel_dir/build. Modules will be installed in
> > $DEST_DIR/$kernel_dir/extra/dpdk.')
> > option('lib_musdk_dir', type: 'string', value: '',
> > description: 'path to the MUSDK library installation directory')
> > -option('machine', type: 'string', value: 'native',
> > - description: 'set the target machine type')
> > +option('machine', type: 'string', value: 'auto',
> > + description: 'set the target machine type/ISA')
> > option('max_ethports', type: 'integer', value: 32,
> > description: 'maximum number of Ethernet devices')
> > option('max_lcores', type: 'integer', value: 128,
> > description: 'maximum number of cores/threads supported by EAL')
> > option('max_numa_nodes', type: 'integer', value: 4,
> > description: 'maximum number of NUMA nodes supported by EAL')
> > +option('platform', type: 'string', value: 'generic',
> > + description: 'Platform to build for, either "native" or "generic".')
>
> As well as this short description option, I think we need more comprehensive
> coverage of this option in the docs.
Where should this be documented? In doc/guides/linux_gsg/build_dpdk.rst or somewhere else?
> Presumably for ARM systems this will have
> other options for various SOC's rather than just generic/native?
>
Yes, I'm planning on using the platform option for this. Since we've separated the changes into their own patch sets, only this small change is in this patch set. It'll affect how we're doing the automatic numa/core detection as well as how we're choosic which Arm SoC to build for, but those changes are in different patch sets.
I mainly want feedback on naming - I like 'platform' and I don't think we need to rename 'machine', maybe just document it better - and on the overall idea. I think this is in line with what you had in mind, right?
> /Bruce
On Fri, Nov 27, 2020 at 08:31:47AM +0000, Juraj Linkeš wrote:
>
>
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Thursday, November 26, 2020 5:03 PM
> > To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > Cc: thomas@monjalon.net; Honnappa.Nagarahalli@arm.com; dev@dpdk.org
> > Subject: Re: [RFC PATCH v1] build: add platform meson option
> >
> > On Thu, Nov 26, 2020 at 04:47:29PM +0100, Juraj Linkeš wrote:
> > > The current meson option 'machine' should only specify the ISA, which
> > > is not sufficient for Arm, where setting ISA implies other setting as well.
> > > Add a new meson option, 'platform', which differentiates the type of
> > > the build (native/generic) and sets machine accordingly, unless the
> > > user chooses to override it.
> > >
> > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > ---
> > > config/arm/meson.build | 2 +-
> > > config/meson.build | 14 +++++++++++++-
> > > meson_options.txt | 6 ++++--
> > > 3 files changed, 18 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > > 42b4e43c7..ac680956f 100644
> > > --- a/config/arm/meson.build
> > > +++ b/config/arm/meson.build
> > > @@ -6,7 +6,7 @@
> > > march_opt = '-march=@0@'.format(machine)
> > >
> > > arm_force_native_march = false
> > > -arm_force_default_march = (machine == 'default')
> > > +arm_force_default_march = (platform == 'generic')
> > >
> > > flags_common_default = [
> > > # Accelarate rte_memcpy. Be sure to run unit test
> > > (memcpy_perf_autotest) diff --git a/config/meson.build
> > > b/config/meson.build index c02802c18..41d32e63e 100644
> > > --- a/config/meson.build
> > > +++ b/config/meson.build
> > > @@ -63,6 +63,8 @@ if not is_windows
> > > pmd_subdir_opt)
> > > endif
> > >
> > > +platform = get_option('platform')
> > > +
> > > # set the machine type and cflags for it if meson.is_cross_build()
> > > machine = host_machine.cpu()
> > > @@ -70,13 +72,23 @@ else
> > > machine = get_option('machine')
> > > endif
> > >
> > > +if platform == 'native'
> > > + if machine == 'auto'
> > > + machine = 'native'
> > > + endif
> > > +elif platform == 'generic'
> > > + if machine == 'auto'
> > > + machine = 'default'
> > > + endif
> > > +endif
> > > +
> > > +if machine == 'default'
> > > # machine type 'default' is special, it defaults to the per arch
> > > agreed common # minimal baseline needed for DPDK.
> > > # That might not be the most optimized, but the most portable version
> > > while # still being able to support the CPU features required for DPDK.
> > > # This can be bumped up by the DPDK project, but it can never be an
> > > # invariant like 'native'
> > > -if machine == 'default'
> > > if host_machine.cpu_family().startswith('x86')
> > > # matches the old pre-meson build systems default
> > > machine = 'corei7'
> > > diff --git a/meson_options.txt b/meson_options.txt index
> > > e384e6dbb..1a5e47fd3 100644
> > > --- a/meson_options.txt
> > > +++ b/meson_options.txt
> > > @@ -20,14 +20,16 @@ option('kernel_dir', type: 'string', value: '',
> > > description: 'Path to the kernel for building kernel modules.
> > > Headers must be in $kernel_dir/build. Modules will be installed in
> > > $DEST_DIR/$kernel_dir/extra/dpdk.')
> > > option('lib_musdk_dir', type: 'string', value: '',
> > > description: 'path to the MUSDK library installation directory')
> > > -option('machine', type: 'string', value: 'native',
> > > - description: 'set the target machine type')
> > > +option('machine', type: 'string', value: 'auto',
> > > + description: 'set the target machine type/ISA')
> > > option('max_ethports', type: 'integer', value: 32,
> > > description: 'maximum number of Ethernet devices')
> > > option('max_lcores', type: 'integer', value: 128,
> > > description: 'maximum number of cores/threads supported by EAL')
> > > option('max_numa_nodes', type: 'integer', value: 4,
> > > description: 'maximum number of NUMA nodes supported by EAL')
> > > +option('platform', type: 'string', value: 'generic',
> > > + description: 'Platform to build for, either "native" or "generic".')
> >
> > As well as this short description option, I think we need more comprehensive
> > coverage of this option in the docs.
>
> Where should this be documented? In doc/guides/linux_gsg/build_dpdk.rst or somewhere else?
>
I'll need to look into this, but I think that we need to expand that
section to cover in more detail all build options as we rework them.
> > Presumably for ARM systems this will have
> > other options for various SOC's rather than just generic/native?
> >
>
> Yes, I'm planning on using the platform option for this. Since we've separated the changes into their own patch sets, only this small change is in this patch set. It'll affect how we're doing the automatic numa/core detection as well as how we're choosic which Arm SoC to build for, but those changes are in different patch sets.
>
> I mainly want feedback on naming - I like 'platform' and I don't think we need to rename 'machine', maybe just document it better - and on the overall idea. I think this is in line with what you had in mind, right?
>
Yes this is in line with what I had in mind, thanks for doing the work.
I'm still of a mind to rename "machine", since it's a very ambiguous term.
I think something like "cpu_instruction_set" would be a lot clearer. With
choice of a good name, the amount of documentation needed around it is much
reduced.
We should probably get feedback from a number of people before making a
decision.
Regards,
/Bruce
> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, November 27, 2020 3:07 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: thomas@monjalon.net; Honnappa.Nagarahalli@arm.com; dev@dpdk.org
> Subject: Re: [RFC PATCH v1] build: add platform meson option
>
> On Fri, Nov 27, 2020 at 08:31:47AM +0000, Juraj Linkeš wrote:
> >
> >
> > > -----Original Message-----
> > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > Sent: Thursday, November 26, 2020 5:03 PM
> > > To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > Cc: thomas@monjalon.net; Honnappa.Nagarahalli@arm.com;
> dev@dpdk.org
> > > Subject: Re: [RFC PATCH v1] build: add platform meson option
> > >
> > > On Thu, Nov 26, 2020 at 04:47:29PM +0100, Juraj Linkeš wrote:
> > > > The current meson option 'machine' should only specify the ISA,
> > > > which is not sufficient for Arm, where setting ISA implies other setting as
> well.
> > > > Add a new meson option, 'platform', which differentiates the type
> > > > of the build (native/generic) and sets machine accordingly, unless
> > > > the user chooses to override it.
> > > >
> > > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > > ---
> > > > config/arm/meson.build | 2 +-
> > > > config/meson.build | 14 +++++++++++++-
> > > > meson_options.txt | 6 ++++--
> > > > 3 files changed, 18 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > > > 42b4e43c7..ac680956f 100644
> > > > --- a/config/arm/meson.build
> > > > +++ b/config/arm/meson.build
> > > > @@ -6,7 +6,7 @@
> > > > march_opt = '-march=@0@'.format(machine)
> > > >
> > > > arm_force_native_march = false
> > > > -arm_force_default_march = (machine == 'default')
> > > > +arm_force_default_march = (platform == 'generic')
> > > >
> > > > flags_common_default = [
> > > > # Accelarate rte_memcpy. Be sure to run unit test
> > > > (memcpy_perf_autotest) diff --git a/config/meson.build
> > > > b/config/meson.build index c02802c18..41d32e63e 100644
> > > > --- a/config/meson.build
> > > > +++ b/config/meson.build
> > > > @@ -63,6 +63,8 @@ if not is_windows
> > > > pmd_subdir_opt)
> > > > endif
> > > >
> > > > +platform = get_option('platform')
> > > > +
> > > > # set the machine type and cflags for it if meson.is_cross_build()
> > > > machine = host_machine.cpu()
> > > > @@ -70,13 +72,23 @@ else
> > > > machine = get_option('machine')
> > > > endif
> > > >
> > > > +if platform == 'native'
> > > > + if machine == 'auto'
> > > > + machine = 'native'
> > > > + endif
> > > > +elif platform == 'generic'
> > > > + if machine == 'auto'
> > > > + machine = 'default'
> > > > + endif
> > > > +endif
> > > > +
> > > > +if machine == 'default'
> > > > # machine type 'default' is special, it defaults to the per arch
> > > > agreed common # minimal baseline needed for DPDK.
> > > > # That might not be the most optimized, but the most portable
> > > > version while # still being able to support the CPU features required for
> DPDK.
> > > > # This can be bumped up by the DPDK project, but it can never be
> > > > an # invariant like 'native'
> > > > -if machine == 'default'
> > > > if host_machine.cpu_family().startswith('x86')
> > > > # matches the old pre-meson build systems default
> > > > machine = 'corei7'
> > > > diff --git a/meson_options.txt b/meson_options.txt index
> > > > e384e6dbb..1a5e47fd3 100644
> > > > --- a/meson_options.txt
> > > > +++ b/meson_options.txt
> > > > @@ -20,14 +20,16 @@ option('kernel_dir', type: 'string', value: '',
> > > > description: 'Path to the kernel for building kernel modules.
> > > > Headers must be in $kernel_dir/build. Modules will be installed in
> > > > $DEST_DIR/$kernel_dir/extra/dpdk.')
> > > > option('lib_musdk_dir', type: 'string', value: '',
> > > > description: 'path to the MUSDK library installation directory')
> > > > -option('machine', type: 'string', value: 'native',
> > > > - description: 'set the target machine type')
> > > > +option('machine', type: 'string', value: 'auto',
> > > > + description: 'set the target machine type/ISA')
> > > > option('max_ethports', type: 'integer', value: 32,
> > > > description: 'maximum number of Ethernet devices')
> > > > option('max_lcores', type: 'integer', value: 128,
> > > > description: 'maximum number of cores/threads supported by EAL')
> > > > option('max_numa_nodes', type: 'integer', value: 4,
> > > > description: 'maximum number of NUMA nodes supported by EAL')
> > > > +option('platform', type: 'string', value: 'generic',
> > > > + description: 'Platform to build for, either "native" or
> > > > +"generic".')
> > >
> > > As well as this short description option, I think we need more
> > > comprehensive coverage of this option in the docs.
> >
> > Where should this be documented? In doc/guides/linux_gsg/build_dpdk.rst or
> somewhere else?
> >
> I'll need to look into this, but I think that we need to expand that section to
> cover in more detail all build options as we rework them.
>
Any news on this? If we want to document this in just doc/guides/linux_gsg/build_dpdk.rst I can propose some changes.
> > > Presumably for ARM systems this will have other options for various
> > > SOC's rather than just generic/native?
> > >
> >
> > Yes, I'm planning on using the platform option for this. Since we've separated
> the changes into their own patch sets, only this small change is in this patch set.
> It'll affect how we're doing the automatic numa/core detection as well as how
> we're choosic which Arm SoC to build for, but those changes are in different
> patch sets.
> >
> > I mainly want feedback on naming - I like 'platform' and I don't think we need
> to rename 'machine', maybe just document it better - and on the overall idea. I
> think this is in line with what you had in mind, right?
> >
> Yes this is in line with what I had in mind, thanks for doing the work.
>
> I'm still of a mind to rename "machine", since it's a very ambiguous term.
> I think something like "cpu_instruction_set" would be a lot clearer. With choice
> of a good name, the amount of documentation needed around it is much
> reduced.
>
I wanted to keep machine mainly for backwards compatibilty, but we can do that anyway, as in keep machine for a release a two with a deprecation notice and then remove it while using a differently named (but same in function) option.
What's were doing with the option is setting the appropriate "machine args", as the variable is called. For x86, that means setting -march (which is just the instruction set according to the docs [0]), for ppc it sets -mcpu and -mtune (architecture type, register usage, and instruction scheduling parameters according to [1]) and we're not using it to set machine args for arm at all (we are setting machine args based on other criteria; when this new option along with platform are in effect, we can think about using cpu_instruction_set somehow).
With this in mind, cpu_instruction_set seems like the best candidate. I'll submit a new version with both cpu_instruction_set and machine if there aren't any objections in some time.
[0] https://gcc.gnu.org/onlinedocs/gcc-8.4.0/gcc/x86-Options.html#x86-Options
[1] https://gcc.gnu.org/onlinedocs/gcc-8.4.0/gcc/RS_002f6000-and-PowerPC-Options.html#RS_002f6000-and-PowerPC-Options
> We should probably get feedback from a number of people before making a
> decision.
>
Unfortunately we didn't get any feedback. Do you have anyone in particular in mind?
> Regards,
> /Bruce
@@ -6,7 +6,7 @@
march_opt = '-march=@0@'.format(machine)
arm_force_native_march = false
-arm_force_default_march = (machine == 'default')
+arm_force_default_march = (platform == 'generic')
flags_common_default = [
# Accelarate rte_memcpy. Be sure to run unit test (memcpy_perf_autotest)
@@ -63,6 +63,8 @@ if not is_windows
pmd_subdir_opt)
endif
+platform = get_option('platform')
+
# set the machine type and cflags for it
if meson.is_cross_build()
machine = host_machine.cpu()
@@ -70,13 +72,23 @@ else
machine = get_option('machine')
endif
+if platform == 'native'
+ if machine == 'auto'
+ machine = 'native'
+ endif
+elif platform == 'generic'
+ if machine == 'auto'
+ machine = 'default'
+ endif
+endif
+
+if machine == 'default'
# machine type 'default' is special, it defaults to the per arch agreed common
# minimal baseline needed for DPDK.
# That might not be the most optimized, but the most portable version while
# still being able to support the CPU features required for DPDK.
# This can be bumped up by the DPDK project, but it can never be an
# invariant like 'native'
-if machine == 'default'
if host_machine.cpu_family().startswith('x86')
# matches the old pre-meson build systems default
machine = 'corei7'
@@ -20,14 +20,16 @@ option('kernel_dir', type: 'string', value: '',
description: 'Path to the kernel for building kernel modules. Headers must be in $kernel_dir/build. Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
option('lib_musdk_dir', type: 'string', value: '',
description: 'path to the MUSDK library installation directory')
-option('machine', type: 'string', value: 'native',
- description: 'set the target machine type')
+option('machine', type: 'string', value: 'auto',
+ description: 'set the target machine type/ISA')
option('max_ethports', type: 'integer', value: 32,
description: 'maximum number of Ethernet devices')
option('max_lcores', type: 'integer', value: 128,
description: 'maximum number of cores/threads supported by EAL')
option('max_numa_nodes', type: 'integer', value: 4,
description: 'maximum number of NUMA nodes supported by EAL')
+option('platform', type: 'string', value: 'generic',
+ description: 'Platform to build for, either "native" or "generic".')
option('enable_trace_fp', type: 'boolean', value: false,
description: 'enable fast path trace points.')
option('tests', type: 'boolean', value: true,