[v2,2/3] config: add arm neoverse N1 SDP configuration

Message ID 1564616902-13861-3-git-send-email-gavin.hu@arm.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series add arm neoverse N1 SDP configuration |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Gavin Hu July 31, 2019, 11:48 p.m. UTC
  Arm N1 SDP is an infrastructure segment development platform
based on armv8.2-a Neoverse N1 CPU. For more information, refer to:
https://community.arm.com/developer/tools-software/oss-platforms/w/
docs/440/neoverse-n1-sdp

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Steve Capper <steve.capper@arm.com>
---
 config/arm/meson.build                         |  9 ++++++-
 config/defconfig_arm64-neoversen1-linux-gcc    |  1 +
 config/defconfig_arm64-neoversen1-linuxapp-gcc | 15 ++++++++++++
 mk/machine/neoversen1/rte.vars.mk              | 34 ++++++++++++++++++++++++++
 4 files changed, 58 insertions(+), 1 deletion(-)
 create mode 120000 config/defconfig_arm64-neoversen1-linux-gcc
 create mode 100644 config/defconfig_arm64-neoversen1-linuxapp-gcc
 create mode 100644 mk/machine/neoversen1/rte.vars.mk
  

Comments

Ola Liljedahl Oct. 17, 2019, 10:18 p.m. UTC | #1
On Thu, 2019-08-01 at 07:48 +0800, Gavin Hu wrote:
> Arm N1 SDP is an infrastructure segment development platform
> based on armv8.2-a Neoverse N1 CPU. For more information, refer to:
> https://community.arm.com/developer/tools-software/oss-platforms/w/
> docs/440/neoverse-n1-sdp
> 
> Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Steve Capper <steve.capper@arm.com>
> ---
>  config/arm/meson.build                         |  9 ++++++-
>  config/defconfig_arm64-neoversen1-linux-gcc    |  1 +
>  config/defconfig_arm64-neoversen1-linuxapp-gcc | 15 ++++++++++++
>  mk/machine/neoversen1/rte.vars.mk              | 34
> ++++++++++++++++++++++++++
>  4 files changed, 58 insertions(+), 1 deletion(-)
>  create mode 120000 config/defconfig_arm64-neoversen1-linux-gcc
>  create mode 100644 config/defconfig_arm64-neoversen1-linuxapp-gcc
>  create mode 100644 mk/machine/neoversen1/rte.vars.mk
> 
> diff --git a/config/arm/meson.build b/config/arm/meson.build
> index 979018e..995d321 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -63,6 +63,12 @@ flags_armada = [
>  	['RTE_MAX_LCORE', 16]]
>  
>  flags_default_extra = []
> +flags_neoversen1_extra = [
> +	['RTE_MACHINE', '"neoversen1"'],
What does RTE_MACHINE stand for?

Neoverse N1 is a CPU core, not a platform, not a SoC architecture etc.
Many different SoC's can be designed using the N1 core.
Some will be small with few cores and no NUMA, others might be large with many
cores and multi-socket (NUMA) support.
The N1SDP (N1 System Development Platform) is using such a small SoC (specially
designed for the N1DSP, it is not going to show up in other platforms). The
configuration below matches N1SDP but will most likely not match other future
N1-based SoC's. What do we do when other quite different "machines" using the
Neoverse N1 core shows up? Is it OK to qualitatively change these
configurations later?

> +	['RTE_MAX_NUMA_NODES', 1],
> +	['RTE_MAX_LCORE', 4],
> +	['RTE_EAL_NUMA_AWARE_HUGEPAGES', false],
> +	['RTE_LIBRTE_VHOST_NUMA', false]]
>  flags_thunderx_extra = [
>  	['RTE_MACHINE', '"thunderx"'],
>  	['RTE_USE_C11_MEM_MODEL', false]]
> @@ -87,7 +93,8 @@ machine_args_generic = [
>  	['0xd07', ['-mcpu=cortex-a57']],
>  	['0xd08', ['-mcpu=cortex-a72']],
>  	['0xd09', ['-mcpu=cortex-a73']],
> -	['0xd0a', ['-mcpu=cortex-a75']]]
> +	['0xd0a', ['-mcpu=cortex-a75']],
> +	['0xd0c', ['-march=armv8.2-a+crc+crypto', '-mcpu=neoverse-n1'],
> flags_neoversen1_extra]]
>  
>  machine_args_cavium = [
>  	['default', ['-march=armv8-a+crc+crypto','-mcpu=thunderx']],
> diff --git a/config/defconfig_arm64-neoversen1-linux-gcc
> b/config/defconfig_arm64-neoversen1-linux-gcc
> new file mode 120000
> index 0000000..47c96a4
> --- /dev/null
> +++ b/config/defconfig_arm64-neoversen1-linux-gcc
> @@ -0,0 +1 @@
> +defconfig_arm64-neoversen1-linuxapp-gcc
> \ No newline at end of file
!

> diff --git a/config/defconfig_arm64-neoversen1-linuxapp-gcc
> b/config/defconfig_arm64-neoversen1-linuxapp-gcc
> new file mode 100644
> index 0000000..39b9e1f
> --- /dev/null
> +++ b/config/defconfig_arm64-neoversen1-linuxapp-gcc
> > 
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2019 Arm Ltd.
> +#
> +
> +#include "defconfig_arm64-armv8a-linux-gcc"
> +
> +CONFIG_RTE_MACHINE="neoversen1"
This should probably be "n1sdp" as this is the name of the
platform that matches the below configuration.

> +CONFIG_RTE_ARCH_ARM_TUNE="neoverse-n1"
> +CONFIG_RTE_MAX_LCORE=4
> +CONFIG_RTE_MAX_NUMA_NODES=1
> +CONFIG_RTE_CACHE_LINE_SIZE=64
> +
> +# Doesn't support NUMA
> +CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n
> +CONFIG_RTE_LIBRTE_VHOST_NUMA=n
> diff --git a/mk/machine/neoversen1/rte.vars.mk
> b/mk/machine/neoversen1/rte.vars.mk
> new file mode 100644
> index 0000000..6d69de0
> --- /dev/null
> +++ b/mk/machine/neoversen1/rte.vars.mk
> @@ -0,0 +1,34 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2019 Arm Ltd
> +#
> +
> +#
> +# machine:
> +#
> +#   - can define ARCH variable (overridden by cmdline value)
> +#   - can define CROSS variable (overridden by cmdline value)
> +#   - define MACHINE_CFLAGS variable (overridden by cmdline value)
> +#   - define MACHINE_LDFLAGS variable (overridden by cmdline value)
> +#   - define MACHINE_ASFLAGS variable (overridden by cmdline value)
> +#   - can define CPU_CFLAGS variable (overridden by cmdline value) that
> +#     overrides the one defined in arch.
> +#   - can define CPU_LDFLAGS variable (overridden by cmdline value) that
> +#     overrides the one defined in arch.
> +#   - can define CPU_ASFLAGS variable (overridden by cmdline value) that
> +#     overrides the one defined in arch.
> +#   - may override any previously defined variable
> +#
> +
> +# ARCH =
> +# CROSS =
> +# MACHINE_CFLAGS =
> +# MACHINE_LDFLAGS =
> +# MACHINE_ASFLAGS =
> +# CPU_CFLAGS =
> +# CPU_LDFLAGS =
> +# CPU_ASFLAGS =
> +
> +include $(RTE_SDK)/mk/rte.helper.mk
> +
> +MACHINE_CFLAGS += $(call rte_cc_has_argument, -march=armv8.2-a+crc+crypto)
> +MACHINE_CFLAGS += $(call rte_cc_has_argument, -mcpu=neoverse-n1)
> 
>
  
Honnappa Nagarahalli Oct. 18, 2019, 5:12 a.m. UTC | #2
<snip>

> 
> On Thu, 2019-08-01 at 07:48 +0800, Gavin Hu wrote:
> > Arm N1 SDP is an infrastructure segment development platform based on
> > armv8.2-a Neoverse N1 CPU. For more information, refer to:
> > https://community.arm.com/developer/tools-software/oss-platforms/w/
> > docs/440/neoverse-n1-sdp
> >
> > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Reviewed-by: Steve Capper <steve.capper@arm.com>
> > ---
> >  config/arm/meson.build                         |  9 ++++++-
> >  config/defconfig_arm64-neoversen1-linux-gcc    |  1 +
> >  config/defconfig_arm64-neoversen1-linuxapp-gcc | 15 ++++++++++++
> >  mk/machine/neoversen1/rte.vars.mk              | 34
> > ++++++++++++++++++++++++++
> >  4 files changed, 58 insertions(+), 1 deletion(-)  create mode 120000
> > config/defconfig_arm64-neoversen1-linux-gcc
> >  create mode 100644 config/defconfig_arm64-neoversen1-linuxapp-gcc
> >  create mode 100644 mk/machine/neoversen1/rte.vars.mk
> >
> > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > 979018e..995d321 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -63,6 +63,12 @@ flags_armada = [
> >  ['RTE_MAX_LCORE', 16]]
> >
> >  flags_default_extra = []
> > +flags_neoversen1_extra = [
> > +['RTE_MACHINE', '"neoversen1"'],
> What does RTE_MACHINE stand for?
Not sure, what it stands for. I cannot find a consistent assignment for it. Some time it takes 'armv8' (in the case of BlueField), 'dpaa', 'dpaa2', 'thunderx' etc (architecture, SoC, micro-architecture). I also cannot find an instance of its usage.

> 
> Neoverse N1 is a CPU core, not a platform, not a SoC architecture etc.
> Many different SoC's can be designed using the N1 core.
> Some will be small with few cores and no NUMA, others might be large with
> many cores and multi-socket (NUMA) support.
> The N1SDP (N1 System Development Platform) is using such a small SoC
> (specially designed for the N1DSP, it is not going to show up in other
> platforms). The configuration below matches N1SDP but will most likely not
> match other future N1-based SoC's. What do we do when other quite
> different "machines" using the Neoverse N1 core shows up? Is it OK to
> qualitatively change these configurations later?
All other implementations of N1 based SoCs should have their own implementor ID and variant. Hence they should not clash with these configurations.
IIRC, BlueField had the same implementation ID as Arm (0x41), so ' flags_arm' was modified to reflect BlueField configuration. Hopefully, this will be corrected in the future.

> 
> > +['RTE_MAX_NUMA_NODES', 1],
> > +['RTE_MAX_LCORE', 4],
> > +['RTE_EAL_NUMA_AWARE_HUGEPAGES', false],
> ['RTE_LIBRTE_VHOST_NUMA',
> > +false]]
> >  flags_thunderx_extra = [
> >  ['RTE_MACHINE', '"thunderx"'],
> >  ['RTE_USE_C11_MEM_MODEL', false]]
> > @@ -87,7 +93,8 @@ machine_args_generic = [  ['0xd07',
> > ['-mcpu=cortex-a57']],  ['0xd08', ['-mcpu=cortex-a72']],  ['0xd09',
> > ['-mcpu=cortex-a73']], -['0xd0a', ['-mcpu=cortex-a75']]]
> > +['0xd0a', ['-mcpu=cortex-a75']],
> > +['0xd0c', ['-march=armv8.2-a+crc+crypto', '-mcpu=neoverse-n1'],
> > flags_neoversen1_extra]]
> >
> >  machine_args_cavium = [
> >  ['default', ['-march=armv8-a+crc+crypto','-mcpu=thunderx']],
> > diff --git a/config/defconfig_arm64-neoversen1-linux-gcc
> > b/config/defconfig_arm64-neoversen1-linux-gcc
> > new file mode 120000
> > index 0000000..47c96a4
> > --- /dev/null
> > +++ b/config/defconfig_arm64-neoversen1-linux-gcc
> > @@ -0,0 +1 @@
> > +defconfig_arm64-neoversen1-linuxapp-gcc
> > \ No newline at end of file
> !
> 
> > diff --git a/config/defconfig_arm64-neoversen1-linuxapp-gcc
> > b/config/defconfig_arm64-neoversen1-linuxapp-gcc
> > new file mode 100644
> > index 0000000..39b9e1f
> > --- /dev/null
> > +++ b/config/defconfig_arm64-neoversen1-linuxapp-gcc
> > >
> > @@ -0,0 +1,15 @@
> > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2019 Arm Ltd.
> > +#
> > +
> > +#include "defconfig_arm64-armv8a-linux-gcc"
> > +
> > +CONFIG_RTE_MACHINE="neoversen1"
> This should probably be "n1sdp" as this is the name of the platform that
> matches the below configuration.
A clear definition of RTE_MACHINE is required. Jerin?

> 
> > +CONFIG_RTE_ARCH_ARM_TUNE="neoverse-n1"
> > +CONFIG_RTE_MAX_LCORE=4
> > +CONFIG_RTE_MAX_NUMA_NODES=1
> > +CONFIG_RTE_CACHE_LINE_SIZE=64
> > +
> > +# Doesn't support NUMA
> > +CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n
> > +CONFIG_RTE_LIBRTE_VHOST_NUMA=n
> > diff --git a/mk/machine/neoversen1/rte.vars.mk
> > b/mk/machine/neoversen1/rte.vars.mk
> > new file mode 100644
> > index 0000000..6d69de0
> > --- /dev/null
> > +++ b/mk/machine/neoversen1/rte.vars.mk
> > @@ -0,0 +1,34 @@
> > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2019 Arm Ltd #
> > +
> > +#
> > +# machine:
> > +#
> > +#   - can define ARCH variable (overridden by cmdline value)
> > +#   - can define CROSS variable (overridden by cmdline value)
> > +#   - define MACHINE_CFLAGS variable (overridden by cmdline value)
> > +#   - define MACHINE_LDFLAGS variable (overridden by cmdline value)
> > +#   - define MACHINE_ASFLAGS variable (overridden by cmdline value)
> > +#   - can define CPU_CFLAGS variable (overridden by cmdline value) that
> > +#     overrides the one defined in arch.
> > +#   - can define CPU_LDFLAGS variable (overridden by cmdline value) that
> > +#     overrides the one defined in arch.
> > +#   - can define CPU_ASFLAGS variable (overridden by cmdline value) that
> > +#     overrides the one defined in arch.
> > +#   - may override any previously defined variable
> > +#
> > +
> > +# ARCH =
> > +# CROSS =
> > +# MACHINE_CFLAGS =
> > +# MACHINE_LDFLAGS =
> > +# MACHINE_ASFLAGS =
> > +# CPU_CFLAGS =
> > +# CPU_LDFLAGS =
> > +# CPU_ASFLAGS =
> > +
> > +include $(RTE_SDK)/mk/rte.helper.mk
> > +
> > +MACHINE_CFLAGS += $(call rte_cc_has_argument,
> > +-march=armv8.2-a+crc+crypto) MACHINE_CFLAGS += $(call
> > +rte_cc_has_argument, -mcpu=neoverse-n1)
> >
> >
> --
> Ola Liljedahl, Networking System Architect, Arm Phone +46706866373, Skype
> ola.liljedahl
  
Jerin Jacob Oct. 18, 2019, 7:23 a.m. UTC | #3
On Fri, Oct 18, 2019 at 10:42 AM Honnappa Nagarahalli
<Honnappa.Nagarahalli@arm.com> wrote:
>
> <snip>
>
> >
> > On Thu, 2019-08-01 at 07:48 +0800, Gavin Hu wrote:
> > > Arm N1 SDP is an infrastructure segment development platform based on
> > > armv8.2-a Neoverse N1 CPU. For more information, refer to:
> > > https://community.arm.com/developer/tools-software/oss-platforms/w/
> > > docs/440/neoverse-n1-sdp
> > >
> > > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > Reviewed-by: Steve Capper <steve.capper@arm.com>
> > > ---
> > >  config/arm/meson.build                         |  9 ++++++-
> > >  config/defconfig_arm64-neoversen1-linux-gcc    |  1 +
> > >  config/defconfig_arm64-neoversen1-linuxapp-gcc | 15 ++++++++++++
> > >  mk/machine/neoversen1/rte.vars.mk              | 34

> >
> > > diff --git a/config/defconfig_arm64-neoversen1-linuxapp-gcc
> > > b/config/defconfig_arm64-neoversen1-linuxapp-gcc
> > > new file mode 100644
> > > index 0000000..39b9e1f
> > > --- /dev/null
> > > +++ b/config/defconfig_arm64-neoversen1-linuxapp-gcc
> > > >
> > > @@ -0,0 +1,15 @@
> > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2019 Arm Ltd.
> > > +#
> > > +
> > > +#include "defconfig_arm64-armv8a-linux-gcc"
> > > +
> > > +CONFIG_RTE_MACHINE="neoversen1"
> > This should probably be "n1sdp" as this is the name of the platform that
> > matches the below configuration.
> A clear definition of RTE_MACHINE is required. Jerin?

I think, In the existing scheme of things, RTE_MACHINE defines, where
to take the MACHINE_CFLAGS
mk/machine/xxxx/rte.vars.mk

Considering the fact that there will be a lot of reusable IPs(for CPU)
from ARM for  armv8, I think,
it would make sense to introduce  RTE_MICRO_ARCH to avoid a lot of
code duplications and
confusion.

RTE_ARCH                             example: "x86" or "arm64"
RTE_MICRO_ARCH               example: "a72" or "thunderx3" - defines
mcpu and armv8 verion arch etc
RTE_MACHINE                       example: "bluefield" or "thunderx3"
- defines, number of cores, NUMA or not? etc

RTE_MACHINE should be probe based  on "implementation ID" for arm64
and reuse already defined RTE_MICRO_ARCH
to avoid code duplication.


>
> >
> > > +CONFIG_RTE_ARCH_ARM_TUNE="neoverse-n1"
> > > +CONFIG_RTE_MAX_LCORE=4
> > > +CONFIG_RTE_MAX_NUMA_NODES=1
> > > +CONFIG_RTE_CACHE_LINE_SIZE=64
> > > +
> > > +# Doesn't support NUMA
> > > +CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n
> > > +CONFIG_RTE_LIBRTE_VHOST_NUMA=n
> > > diff --git a/mk/machine/neoversen1/rte.vars.mk
> > > b/mk/machine/neoversen1/rte.vars.mk
> > > new file mode 100644
> > > index 0000000..6d69de0
> > > --- /dev/null
> > > +++ b/mk/machine/neoversen1/rte.vars.mk
> > > @@ -0,0 +1,34 @@
> > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2019 Arm Ltd #
> > > +
> > > +#
> > > +# machine:
> > > +#
> > > +#   - can define ARCH variable (overridden by cmdline value)
> > > +#   - can define CROSS variable (overridden by cmdline value)
> > > +#   - define MACHINE_CFLAGS variable (overridden by cmdline value)
> > > +#   - define MACHINE_LDFLAGS variable (overridden by cmdline value)
> > > +#   - define MACHINE_ASFLAGS variable (overridden by cmdline value)
> > > +#   - can define CPU_CFLAGS variable (overridden by cmdline value) that
> > > +#     overrides the one defined in arch.
> > > +#   - can define CPU_LDFLAGS variable (overridden by cmdline value) that
> > > +#     overrides the one defined in arch.
> > > +#   - can define CPU_ASFLAGS variable (overridden by cmdline value) that
> > > +#     overrides the one defined in arch.
> > > +#   - may override any previously defined variable
> > > +#
> > > +
> > > +# ARCH =
> > > +# CROSS =
> > > +# MACHINE_CFLAGS =
> > > +# MACHINE_LDFLAGS =
> > > +# MACHINE_ASFLAGS =
> > > +# CPU_CFLAGS =
> > > +# CPU_LDFLAGS =
> > > +# CPU_ASFLAGS =
> > > +
> > > +include $(RTE_SDK)/mk/rte.helper.mk
> > > +
> > > +MACHINE_CFLAGS += $(call rte_cc_has_argument,
> > > +-march=armv8.2-a+crc+crypto) MACHINE_CFLAGS += $(call
> > > +rte_cc_has_argument, -mcpu=neoverse-n1)
> > >
> > >
> > --
> > Ola Liljedahl, Networking System Architect, Arm Phone +46706866373, Skype
> > ola.liljedahl
>
  
Honnappa Nagarahalli Oct. 22, 2019, 9:07 p.m. UTC | #4
> > <snip>
> >
> > >
> > > On Thu, 2019-08-01 at 07:48 +0800, Gavin Hu wrote:
> > > > Arm N1 SDP is an infrastructure segment development platform based
> > > > on armv8.2-a Neoverse N1 CPU. For more information, refer to:
> > > > https://community.arm.com/developer/tools-software/oss-platforms/w
> > > > /
> > > > docs/440/neoverse-n1-sdp
> > > >
> > > > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > > > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > > Reviewed-by: Steve Capper <steve.capper@arm.com>
> > > > ---
> > > >  config/arm/meson.build                         |  9 ++++++-
> > > >  config/defconfig_arm64-neoversen1-linux-gcc    |  1 +
> > > >  config/defconfig_arm64-neoversen1-linuxapp-gcc | 15 ++++++++++++
> > > >  mk/machine/neoversen1/rte.vars.mk              | 34
> 
> > >
> > > > diff --git a/config/defconfig_arm64-neoversen1-linuxapp-gcc
> > > > b/config/defconfig_arm64-neoversen1-linuxapp-gcc
> > > > new file mode 100644
> > > > index 0000000..39b9e1f
> > > > --- /dev/null
> > > > +++ b/config/defconfig_arm64-neoversen1-linuxapp-gcc
> > > > >
> > > > @@ -0,0 +1,15 @@
> > > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2019 Arm Ltd.
> > > > +#
> > > > +
> > > > +#include "defconfig_arm64-armv8a-linux-gcc"
> > > > +
> > > > +CONFIG_RTE_MACHINE="neoversen1"
> > > This should probably be "n1sdp" as this is the name of the platform
> > > that matches the below configuration.
> > A clear definition of RTE_MACHINE is required. Jerin?
> 
> I think, In the existing scheme of things, RTE_MACHINE defines, where to take
> the MACHINE_CFLAGS mk/machine/xxxx/rte.vars.mk
Ok, thank you

> 
> Considering the fact that there will be a lot of reusable IPs(for CPU) from ARM
> for  armv8, I think, it would make sense to introduce  RTE_MICRO_ARCH to
> avoid a lot of code duplications and confusion.
> 
> RTE_ARCH                             example: "x86" or "arm64"
> RTE_MICRO_ARCH               example: "a72" or "thunderx3" - defines
> mcpu and armv8 verion arch etc
> RTE_MACHINE                       example: "bluefield" or "thunderx3"
> - defines, number of cores, NUMA or not? etc
Looking at mk/machine/ directory, looks like RTE_MACHINE seems to be defining micro-architecture for Intel. For ex: hsw, nhm, wsm. I see the same for Arm as well.
Are you suggesting that we use RTE_MICRO_ARCH to pick mk/micro-arch/xxxx/rte.vars.mk? and RTE_MACHINE would pick mk/machine/xxxx/rte.vars.mk, but contain NUMA, #of cores etc?

> 
> RTE_MACHINE should be probe based  on "implementation ID" for arm64 and
> reuse already defined RTE_MICRO_ARCH to avoid code duplication.
> 
> 
> >
> > >
> > > > +CONFIG_RTE_ARCH_ARM_TUNE="neoverse-n1"
> > > > +CONFIG_RTE_MAX_LCORE=4
> > > > +CONFIG_RTE_MAX_NUMA_NODES=1
> > > > +CONFIG_RTE_CACHE_LINE_SIZE=64
> > > > +
> > > > +# Doesn't support NUMA
> > > > +CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n
> > > > +CONFIG_RTE_LIBRTE_VHOST_NUMA=n
> > > > diff --git a/mk/machine/neoversen1/rte.vars.mk
> > > > b/mk/machine/neoversen1/rte.vars.mk
> > > > new file mode 100644
> > > > index 0000000..6d69de0
> > > > --- /dev/null
> > > > +++ b/mk/machine/neoversen1/rte.vars.mk
> > > > @@ -0,0 +1,34 @@
> > > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2019 Arm
> > > > +Ltd #
> > > > +
> > > > +#
> > > > +# machine:
> > > > +#
> > > > +#   - can define ARCH variable (overridden by cmdline value)
> > > > +#   - can define CROSS variable (overridden by cmdline value)
> > > > +#   - define MACHINE_CFLAGS variable (overridden by cmdline value)
> > > > +#   - define MACHINE_LDFLAGS variable (overridden by cmdline value)
> > > > +#   - define MACHINE_ASFLAGS variable (overridden by cmdline value)
> > > > +#   - can define CPU_CFLAGS variable (overridden by cmdline value) that
> > > > +#     overrides the one defined in arch.
> > > > +#   - can define CPU_LDFLAGS variable (overridden by cmdline value) that
> > > > +#     overrides the one defined in arch.
> > > > +#   - can define CPU_ASFLAGS variable (overridden by cmdline value) that
> > > > +#     overrides the one defined in arch.
> > > > +#   - may override any previously defined variable
> > > > +#
> > > > +
> > > > +# ARCH =
> > > > +# CROSS =
> > > > +# MACHINE_CFLAGS =
> > > > +# MACHINE_LDFLAGS =
> > > > +# MACHINE_ASFLAGS =
> > > > +# CPU_CFLAGS =
> > > > +# CPU_LDFLAGS =
> > > > +# CPU_ASFLAGS =
> > > > +
> > > > +include $(RTE_SDK)/mk/rte.helper.mk
> > > > +
> > > > +MACHINE_CFLAGS += $(call rte_cc_has_argument,
> > > > +-march=armv8.2-a+crc+crypto) MACHINE_CFLAGS += $(call
> > > > +rte_cc_has_argument, -mcpu=neoverse-n1)
> > > >
> > > >
> > > --
> > > Ola Liljedahl, Networking System Architect, Arm Phone +46706866373,
> > > Skype ola.liljedahl
> >
  
Jerin Jacob Oct. 23, 2019, 5:03 a.m. UTC | #5
On Wed, Oct 23, 2019 at 2:37 AM Honnappa Nagarahalli
<Honnappa.Nagarahalli@arm.com> wrote:
>
> > > <snip>
> > >
> > > >
> > > > On Thu, 2019-08-01 at 07:48 +0800, Gavin Hu wrote:
> > > > > Arm N1 SDP is an infrastructure segment development platform based
> > > > > on armv8.2-a Neoverse N1 CPU. For more information, refer to:
> > > > > https://community.arm.com/developer/tools-software/oss-platforms/w
> > > > > /
> > > > > docs/440/neoverse-n1-sdp
> > > > >
> > > > > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > > > > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > > > Reviewed-by: Steve Capper <steve.capper@arm.com>
> > > > > ---
> > > > >  config/arm/meson.build                         |  9 ++++++-
> > > > >  config/defconfig_arm64-neoversen1-linux-gcc    |  1 +
> > > > >  config/defconfig_arm64-neoversen1-linuxapp-gcc | 15 ++++++++++++
> > > > >  mk/machine/neoversen1/rte.vars.mk              | 34
> >
> > > >
> > > > > diff --git a/config/defconfig_arm64-neoversen1-linuxapp-gcc
> > > > > b/config/defconfig_arm64-neoversen1-linuxapp-gcc
> > > > > new file mode 100644
> > > > > index 0000000..39b9e1f
> > > > > --- /dev/null
> > > > > +++ b/config/defconfig_arm64-neoversen1-linuxapp-gcc
> > > > > >
> > > > > @@ -0,0 +1,15 @@
> > > > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2019 Arm Ltd.
> > > > > +#
> > > > > +
> > > > > +#include "defconfig_arm64-armv8a-linux-gcc"
> > > > > +
> > > > > +CONFIG_RTE_MACHINE="neoversen1"
> > > > This should probably be "n1sdp" as this is the name of the platform
> > > > that matches the below configuration.
> > > A clear definition of RTE_MACHINE is required. Jerin?
> >
> > I think, In the existing scheme of things, RTE_MACHINE defines, where to take
> > the MACHINE_CFLAGS mk/machine/xxxx/rte.vars.mk
> Ok, thank you
>
> >
> > Considering the fact that there will be a lot of reusable IPs(for CPU) from ARM
> > for  armv8, I think, it would make sense to introduce  RTE_MICRO_ARCH to
> > avoid a lot of code duplications and confusion.
> >
> > RTE_ARCH                             example: "x86" or "arm64"
> > RTE_MICRO_ARCH               example: "a72" or "thunderx3" - defines
> > mcpu and armv8 verion arch etc
> > RTE_MACHINE                       example: "bluefield" or "thunderx3"
> > - defines, number of cores, NUMA or not? etc
> Looking at mk/machine/ directory, looks like RTE_MACHINE seems to be defining micro-architecture for Intel. For ex: hsw, nhm, wsm. I see the same for Arm as well.
> Are you suggesting that we use RTE_MICRO_ARCH to pick mk/micro-arch/xxxx/rte.vars.mk? and RTE_MACHINE would pick mk/machine/xxxx/rte.vars.mk, but contain NUMA, #of cores etc?

Yes for Make build. I think, it is deprecated soon, so we need a
similar solution for meson.

>
> >
> > RTE_MACHINE should be probe based  on "implementation ID" for arm64 and
> > reuse already defined RTE_MICRO_ARCH to avoid code duplication.
> >
> >
> > >
> > > >
> > > > > +CONFIG_RTE_ARCH_ARM_TUNE="neoverse-n1"
> > > > > +CONFIG_RTE_MAX_LCORE=4
> > > > > +CONFIG_RTE_MAX_NUMA_NODES=1
> > > > > +CONFIG_RTE_CACHE_LINE_SIZE=64
> > > > > +
> > > > > +# Doesn't support NUMA
> > > > > +CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n
> > > > > +CONFIG_RTE_LIBRTE_VHOST_NUMA=n
> > > > > diff --git a/mk/machine/neoversen1/rte.vars.mk
> > > > > b/mk/machine/neoversen1/rte.vars.mk
> > > > > new file mode 100644
> > > > > index 0000000..6d69de0
> > > > > --- /dev/null
> > > > > +++ b/mk/machine/neoversen1/rte.vars.mk
> > > > > @@ -0,0 +1,34 @@
> > > > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2019 Arm
> > > > > +Ltd #
> > > > > +
> > > > > +#
> > > > > +# machine:
> > > > > +#
> > > > > +#   - can define ARCH variable (overridden by cmdline value)
> > > > > +#   - can define CROSS variable (overridden by cmdline value)
> > > > > +#   - define MACHINE_CFLAGS variable (overridden by cmdline value)
> > > > > +#   - define MACHINE_LDFLAGS variable (overridden by cmdline value)
> > > > > +#   - define MACHINE_ASFLAGS variable (overridden by cmdline value)
> > > > > +#   - can define CPU_CFLAGS variable (overridden by cmdline value) that
> > > > > +#     overrides the one defined in arch.
> > > > > +#   - can define CPU_LDFLAGS variable (overridden by cmdline value) that
> > > > > +#     overrides the one defined in arch.
> > > > > +#   - can define CPU_ASFLAGS variable (overridden by cmdline value) that
> > > > > +#     overrides the one defined in arch.
> > > > > +#   - may override any previously defined variable
> > > > > +#
> > > > > +
> > > > > +# ARCH =
> > > > > +# CROSS =
> > > > > +# MACHINE_CFLAGS =
> > > > > +# MACHINE_LDFLAGS =
> > > > > +# MACHINE_ASFLAGS =
> > > > > +# CPU_CFLAGS =
> > > > > +# CPU_LDFLAGS =
> > > > > +# CPU_ASFLAGS =
> > > > > +
> > > > > +include $(RTE_SDK)/mk/rte.helper.mk
> > > > > +
> > > > > +MACHINE_CFLAGS += $(call rte_cc_has_argument,
> > > > > +-march=armv8.2-a+crc+crypto) MACHINE_CFLAGS += $(call
> > > > > +rte_cc_has_argument, -mcpu=neoverse-n1)
> > > > >
> > > > >
> > > > --
> > > > Ola Liljedahl, Networking System Architect, Arm Phone +46706866373,
> > > > Skype ola.liljedahl
> > >
  
Thomas Monjalon Oct. 27, 2019, 9:22 p.m. UTC | #6
23/10/2019 07:03, Jerin Jacob:
> On Wed, Oct 23, 2019 at 2:37 AM Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com> wrote:
> > > > > On Thu, 2019-08-01 at 07:48 +0800, Gavin Hu wrote:
> > > > > > Arm N1 SDP is an infrastructure segment development platform based
> > > > > > on armv8.2-a Neoverse N1 CPU. For more information, refer to:
> > > > > > https://community.arm.com/developer/tools-software/oss-platforms/w
> > > > > > /
> > > > > > docs/440/neoverse-n1-sdp
> > > > > >
> > > > > > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > > > > > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > > > > Reviewed-by: Steve Capper <steve.capper@arm.com>
> > > > > > ---
> > > > > > +CONFIG_RTE_MACHINE="neoversen1"
> > > > > This should probably be "n1sdp" as this is the name of the platform
> > > > > that matches the below configuration.
> > > > A clear definition of RTE_MACHINE is required. Jerin?
> > >
> > > I think, In the existing scheme of things, RTE_MACHINE defines, where to take
> > > the MACHINE_CFLAGS mk/machine/xxxx/rte.vars.mk
> > Ok, thank you
> >
> > >
> > > Considering the fact that there will be a lot of reusable IPs(for CPU) from ARM
> > > for  armv8, I think, it would make sense to introduce  RTE_MICRO_ARCH to
> > > avoid a lot of code duplications and confusion.
> > >
> > > RTE_ARCH                             example: "x86" or "arm64"
> > > RTE_MICRO_ARCH               example: "a72" or "thunderx3" - defines
> > > mcpu and armv8 verion arch etc
> > > RTE_MACHINE                       example: "bluefield" or "thunderx3"
> > > - defines, number of cores, NUMA or not? etc
> > Looking at mk/machine/ directory, looks like RTE_MACHINE seems to be defining micro-architecture for Intel. For ex: hsw, nhm, wsm. I see the same for Arm as well.
> > Are you suggesting that we use RTE_MICRO_ARCH to pick mk/micro-arch/xxxx/rte.vars.mk? and RTE_MACHINE would pick mk/machine/xxxx/rte.vars.mk, but contain NUMA, #of cores etc?
> 
> Yes for Make build. I think, it is deprecated soon, so we need a
> similar solution for meson.

Yes I would prefer we clean the mess in Meson,
instead of talking about the makefile system.
And honestly, N1 is not needed in the legacy makefile system.

So focusing on config/arm/meson.build,
I think RTE_MACHINE is defined only for API compatibility with makefile.
However, I doubt this value is used by any application.
I think we can try to remove RTE_MACHINE from meson builds in DPDK 19.11,
or use RTE_MACHINE as micro-arch (my preference).
  
Honnappa Nagarahalli Oct. 28, 2019, 3:24 a.m. UTC | #7
> 23/10/2019 07:03, Jerin Jacob:
> > On Wed, Oct 23, 2019 at 2:37 AM Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com> wrote:
> > > > > > On Thu, 2019-08-01 at 07:48 +0800, Gavin Hu wrote:
> > > > > > > Arm N1 SDP is an infrastructure segment development platform
> > > > > > > based on armv8.2-a Neoverse N1 CPU. For more information, refer
> to:
> > > > > > > https://community.arm.com/developer/tools-software/oss-platf
> > > > > > > orms/w
> > > > > > > /
> > > > > > > docs/440/neoverse-n1-sdp
> > > > > > >
> > > > > > > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > > > > > > Reviewed-by: Honnappa Nagarahalli
> > > > > > > <honnappa.nagarahalli@arm.com>
> > > > > > > Reviewed-by: Steve Capper <steve.capper@arm.com>
> > > > > > > ---
> > > > > > > +CONFIG_RTE_MACHINE="neoversen1"
> > > > > > This should probably be "n1sdp" as this is the name of the
> > > > > > platform that matches the below configuration.
> > > > > A clear definition of RTE_MACHINE is required. Jerin?
> > > >
> > > > I think, In the existing scheme of things, RTE_MACHINE defines,
> > > > where to take the MACHINE_CFLAGS mk/machine/xxxx/rte.vars.mk
> > > Ok, thank you
> > >
> > > >
> > > > Considering the fact that there will be a lot of reusable IPs(for
> > > > CPU) from ARM for  armv8, I think, it would make sense to
> > > > introduce  RTE_MICRO_ARCH to avoid a lot of code duplications and
> confusion.
> > > >
> > > > RTE_ARCH                             example: "x86" or "arm64"
> > > > RTE_MICRO_ARCH               example: "a72" or "thunderx3" - defines
> > > > mcpu and armv8 verion arch etc
> > > > RTE_MACHINE                       example: "bluefield" or "thunderx3"
> > > > - defines, number of cores, NUMA or not? etc
> > > Looking at mk/machine/ directory, looks like RTE_MACHINE seems to be
> defining micro-architecture for Intel. For ex: hsw, nhm, wsm. I see the same
> for Arm as well.
> > > Are you suggesting that we use RTE_MICRO_ARCH to pick mk/micro-
> arch/xxxx/rte.vars.mk? and RTE_MACHINE would pick
> mk/machine/xxxx/rte.vars.mk, but contain NUMA, #of cores etc?
> >
> > Yes for Make build. I think, it is deprecated soon, so we need a
> > similar solution for meson.
> 
> Yes I would prefer we clean the mess in Meson, instead of talking about the
> makefile system.
> And honestly, N1 is not needed in the legacy makefile system.
Unfortunately, most of the guys I talk to are still on makefile. When is makefile system getting removed?

> 
> So focusing on config/arm/meson.build,
> I think RTE_MACHINE is defined only for API compatibility with makefile.
> However, I doubt this value is used by any application.
> I think we can try to remove RTE_MACHINE from meson builds in DPDK 19.11,
> or use RTE_MACHINE as micro-arch (my preference).
'MACHINE' means different things to different people, which is the root cause of this discussion.
'MICRO-ARCH' has a very clear meaning. Do you see any problem going with MICRO-ARCH instead?

>
  
Thomas Monjalon Oct. 28, 2019, 8:36 a.m. UTC | #8
28/10/2019 04:24, Honnappa Nagarahalli:
> > 23/10/2019 07:03, Jerin Jacob:
> > > On Wed, Oct 23, 2019 at 2:37 AM Honnappa Nagarahalli
> > > <Honnappa.Nagarahalli@arm.com> wrote:
> > > > > > > On Thu, 2019-08-01 at 07:48 +0800, Gavin Hu wrote:
> > > > > > > > Arm N1 SDP is an infrastructure segment development platform
> > > > > > > > based on armv8.2-a Neoverse N1 CPU. For more information, refer
> > to:
> > > > > > > > https://community.arm.com/developer/tools-software/oss-platf
> > > > > > > > orms/w
> > > > > > > > /
> > > > > > > > docs/440/neoverse-n1-sdp
> > > > > > > >
> > > > > > > > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > > > > > > > Reviewed-by: Honnappa Nagarahalli
> > > > > > > > <honnappa.nagarahalli@arm.com>
> > > > > > > > Reviewed-by: Steve Capper <steve.capper@arm.com>
> > > > > > > > ---
> > > > > > > > +CONFIG_RTE_MACHINE="neoversen1"
> > > > > > > This should probably be "n1sdp" as this is the name of the
> > > > > > > platform that matches the below configuration.
> > > > > > A clear definition of RTE_MACHINE is required. Jerin?
> > > > >
> > > > > I think, In the existing scheme of things, RTE_MACHINE defines,
> > > > > where to take the MACHINE_CFLAGS mk/machine/xxxx/rte.vars.mk
> > > > Ok, thank you
> > > >
> > > > >
> > > > > Considering the fact that there will be a lot of reusable IPs(for
> > > > > CPU) from ARM for  armv8, I think, it would make sense to
> > > > > introduce  RTE_MICRO_ARCH to avoid a lot of code duplications and
> > confusion.
> > > > >
> > > > > RTE_ARCH                             example: "x86" or "arm64"
> > > > > RTE_MICRO_ARCH               example: "a72" or "thunderx3" - defines
> > > > > mcpu and armv8 verion arch etc
> > > > > RTE_MACHINE                       example: "bluefield" or "thunderx3"
> > > > > - defines, number of cores, NUMA or not? etc
> > > > Looking at mk/machine/ directory, looks like RTE_MACHINE seems to be
> > defining micro-architecture for Intel. For ex: hsw, nhm, wsm. I see the same
> > for Arm as well.
> > > > Are you suggesting that we use RTE_MICRO_ARCH to pick mk/micro-
> > arch/xxxx/rte.vars.mk? and RTE_MACHINE would pick
> > mk/machine/xxxx/rte.vars.mk, but contain NUMA, #of cores etc?
> > >
> > > Yes for Make build. I think, it is deprecated soon, so we need a
> > > similar solution for meson.
> > 
> > Yes I would prefer we clean the mess in Meson, instead of talking about the
> > makefile system.
> > And honestly, N1 is not needed in the legacy makefile system.
> Unfortunately, most of the guys I talk to are still on makefile.

You need to help them to switch.
Adding new targets in meson-only can be a good motivation :)

> When is makefile system getting removed?

It must be clearly decided and announced.
The previous techboard discussions were about making makefile
hardly usable during 2020, i.e. very soon.

> > So focusing on config/arm/meson.build,
> > I think RTE_MACHINE is defined only for API compatibility with makefile.
> > However, I doubt this value is used by any application.
> > I think we can try to remove RTE_MACHINE from meson builds in DPDK 19.11,
> > or use RTE_MACHINE as micro-arch (my preference).
> 'MACHINE' means different things to different people, which is the root cause of this discussion.
> 'MICRO-ARCH' has a very clear meaning. Do you see any problem going with MICRO-ARCH instead?

Some applications may use RTE_MACHINE for this purpose.
It is part of the API since the befinning of DPDK.
I don't see a real motivation to break this API now.
  
Honnappa Nagarahalli Oct. 29, 2019, 5:47 a.m. UTC | #9
<snip>

> 
> 28/10/2019 04:24, Honnappa Nagarahalli:
> > > 23/10/2019 07:03, Jerin Jacob:
> > > > On Wed, Oct 23, 2019 at 2:37 AM Honnappa Nagarahalli
> > > > <Honnappa.Nagarahalli@arm.com> wrote:
> > > > > > > > On Thu, 2019-08-01 at 07:48 +0800, Gavin Hu wrote:
> > > > > > > > > Arm N1 SDP is an infrastructure segment development
> > > > > > > > > platform based on armv8.2-a Neoverse N1 CPU. For more
> > > > > > > > > information, refer
> > > to:
> > > > > > > > > https://community.arm.com/developer/tools-software/oss-p
> > > > > > > > > latf
> > > > > > > > > orms/w
> > > > > > > > > /
> > > > > > > > > docs/440/neoverse-n1-sdp
> > > > > > > > >
> > > > > > > > > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > > > > > > > > Reviewed-by: Honnappa Nagarahalli
> > > > > > > > > <honnappa.nagarahalli@arm.com>
> > > > > > > > > Reviewed-by: Steve Capper <steve.capper@arm.com>
> > > > > > > > > ---
> > > > > > > > > +CONFIG_RTE_MACHINE="neoversen1"
> > > > > > > > This should probably be "n1sdp" as this is the name of the
> > > > > > > > platform that matches the below configuration.
> > > > > > > A clear definition of RTE_MACHINE is required. Jerin?
> > > > > >
> > > > > > I think, In the existing scheme of things, RTE_MACHINE
> > > > > > defines, where to take the MACHINE_CFLAGS
> > > > > > mk/machine/xxxx/rte.vars.mk
> > > > > Ok, thank you
> > > > >
> > > > > >
> > > > > > Considering the fact that there will be a lot of reusable
> > > > > > IPs(for
> > > > > > CPU) from ARM for  armv8, I think, it would make sense to
> > > > > > introduce  RTE_MICRO_ARCH to avoid a lot of code duplications
> > > > > > and
> > > confusion.
> > > > > >
> > > > > > RTE_ARCH                             example: "x86" or "arm64"
I see that there are already RTE_ARCH_X86, RTE_ARCH_ARM, RTE_ARCH_ARM64, RTE_ARCH_PPC_64 etc. I believe they should be sufficient.

> > > > > > RTE_MICRO_ARCH               example: "a72" or "thunderx3" - defines
> > > > > > mcpu and armv8 verion arch etc
Are you suggesting this just for Arm platforms?
My understanding is your intention was to clean up the config/arm/meson.build file.

> > > > > > RTE_MACHINE                       example: "bluefield" or "thunderx3"
> > > > > > - defines, number of cores, NUMA or not? etc
> > > > > Looking at mk/machine/ directory, looks like RTE_MACHINE seems
> > > > > to be
> > > defining micro-architecture for Intel. For ex: hsw, nhm, wsm. I see
> > > the same for Arm as well.
> > > > > Are you suggesting that we use RTE_MICRO_ARCH to pick mk/micro-
> > > arch/xxxx/rte.vars.mk? and RTE_MACHINE would pick
> > > mk/machine/xxxx/rte.vars.mk, but contain NUMA, #of cores etc?
> > > >
> > > > Yes for Make build. I think, it is deprecated soon, so we need a
> > > > similar solution for meson.
> > >
> > > Yes I would prefer we clean the mess in Meson, instead of talking
> > > about the makefile system.
> > > And honestly, N1 is not needed in the legacy makefile system.
> > Unfortunately, most of the guys I talk to are still on makefile.
> 
> You need to help them to switch.
> Adding new targets in meson-only can be a good motivation :)
> 
> > When is makefile system getting removed?
> 
> It must be clearly decided and announced.
> The previous techboard discussions were about making makefile hardly
> usable during 2020, i.e. very soon.
> 
> > > So focusing on config/arm/meson.build, I think RTE_MACHINE is
> > > defined only for API compatibility with makefile.
> > > However, I doubt this value is used by any application.
> > > I think we can try to remove RTE_MACHINE from meson builds in DPDK
> > > 19.11, or use RTE_MACHINE as micro-arch (my preference).
> > 'MACHINE' means different things to different people, which is the root
> cause of this discussion.
> > 'MICRO-ARCH' has a very clear meaning. Do you see any problem going
> with MICRO-ARCH instead?
> 
> Some applications may use RTE_MACHINE for this purpose.
> It is part of the API since the befinning of DPDK.
> I don't see a real motivation to break this API now.
The suggestions are not clear to me. The original suggestion was to introduce RTE_MICRO_ARCH and contain all the micro-architecture related compiler flags in that.
Now, the suggestion is to use RTE_MACHINE to contain micro-architecture related compiler flags. Will it contain NUMA, number of cores as well (as suggested earlier)? If yes, I do not see it changing anything.

I am not a meson expert. However, I looked at various meson.build files. I have few questions/concerns.
1) Are these suggestions are for all the platforms? IMO, these need to be the same across all the architectures.
2) I am looking at config/meson.build. Here RTE_MACHINE is defined to indicate Architecture for Arm (armv7-a, aarchxx) and micro-architecture for x86 (corei7?). Is the understanding correct? IMO, this should this have a common meaning across all the platforms?
3) If the changes are for all the platforms, is the risk high for 19.11 release?
4) The N1 config patch as such conforms to the current conventions. What is being asked here is an enhancement, is the understanding correct?

>
  
Gavin Hu Nov. 11, 2019, 5:06 a.m. UTC | #10
Hi Honnappa,

> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Tuesday, October 29, 2019 1:47 PM
> To: thomas@monjalon.net
> Cc: Jerin Jacob <jerinjacobk@gmail.com>; Ola Liljedahl
> <Ola.Liljedahl@arm.com>; Gavin Hu (Arm Technology China)
> <Gavin.Hu@arm.com>; dev@dpdk.org; pbhagavatula@marvell.com; nd
> <nd@arm.com>; jerinj@marvell.com; hemant.agrawal@nxp.com;
> bruce.richardson@intel.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>
> Subject: RE: [dpdk-dev] [PATCH v2 2/3] config: add arm neoverse N1 SDP
> configuration
> 
> <snip>
> 
> >
> > 28/10/2019 04:24, Honnappa Nagarahalli:
> > > > 23/10/2019 07:03, Jerin Jacob:
> > > > > On Wed, Oct 23, 2019 at 2:37 AM Honnappa Nagarahalli
> > > > > <Honnappa.Nagarahalli@arm.com> wrote:
> > > > > > > > > On Thu, 2019-08-01 at 07:48 +0800, Gavin Hu wrote:
> > > > > > > > > > Arm N1 SDP is an infrastructure segment development
> > > > > > > > > > platform based on armv8.2-a Neoverse N1 CPU. For more
> > > > > > > > > > information, refer
> > > > to:
> > > > > > > > > > https://community.arm.com/developer/tools-software/oss-
> p
> > > > > > > > > > latf
> > > > > > > > > > orms/w
> > > > > > > > > > /
> > > > > > > > > > docs/440/neoverse-n1-sdp
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > > > > > > > > > Reviewed-by: Honnappa Nagarahalli
> > > > > > > > > > <honnappa.nagarahalli@arm.com>
> > > > > > > > > > Reviewed-by: Steve Capper <steve.capper@arm.com>
> > > > > > > > > > ---
> > > > > > > > > > +CONFIG_RTE_MACHINE="neoversen1"
> > > > > > > > > This should probably be "n1sdp" as this is the name of the
> > > > > > > > > platform that matches the below configuration.
> > > > > > > > A clear definition of RTE_MACHINE is required. Jerin?
> > > > > > >
> > > > > > > I think, In the existing scheme of things, RTE_MACHINE
> > > > > > > defines, where to take the MACHINE_CFLAGS
> > > > > > > mk/machine/xxxx/rte.vars.mk
> > > > > > Ok, thank you
> > > > > >
> > > > > > >
> > > > > > > Considering the fact that there will be a lot of reusable
> > > > > > > IPs(for
> > > > > > > CPU) from ARM for  armv8, I think, it would make sense to
> > > > > > > introduce  RTE_MICRO_ARCH to avoid a lot of code duplications
> > > > > > > and
> > > > confusion.
> > > > > > >
> > > > > > > RTE_ARCH                             example: "x86" or "arm64"
> I see that there are already RTE_ARCH_X86, RTE_ARCH_ARM,
> RTE_ARCH_ARM64, RTE_ARCH_PPC_64 etc. I believe they should be
> sufficient.
> 
> > > > > > > RTE_MICRO_ARCH               example: "a72" or "thunderx3" -
> defines
> > > > > > > mcpu and armv8 verion arch etc
> Are you suggesting this just for Arm platforms?
> My understanding is your intention was to clean up the
> config/arm/meson.build file.
> 
> > > > > > > RTE_MACHINE                       example: "bluefield" or "thunderx3"
> > > > > > > - defines, number of cores, NUMA or not? etc
> > > > > > Looking at mk/machine/ directory, looks like RTE_MACHINE seems
> > > > > > to be
> > > > defining micro-architecture for Intel. For ex: hsw, nhm, wsm. I see
> > > > the same for Arm as well.
> > > > > > Are you suggesting that we use RTE_MICRO_ARCH to pick
> mk/micro-
> > > > arch/xxxx/rte.vars.mk? and RTE_MACHINE would pick
> > > > mk/machine/xxxx/rte.vars.mk, but contain NUMA, #of cores etc?
> > > > >
> > > > > Yes for Make build. I think, it is deprecated soon, so we need a
> > > > > similar solution for meson.
> > > >
> > > > Yes I would prefer we clean the mess in Meson, instead of talking
> > > > about the makefile system.
> > > > And honestly, N1 is not needed in the legacy makefile system.
> > > Unfortunately, most of the guys I talk to are still on makefile.
> >
> > You need to help them to switch.
> > Adding new targets in meson-only can be a good motivation :)
> >
> > > When is makefile system getting removed?
> >
> > It must be clearly decided and announced.
> > The previous techboard discussions were about making makefile hardly
> > usable during 2020, i.e. very soon.
> >
> > > > So focusing on config/arm/meson.build, I think RTE_MACHINE is
> > > > defined only for API compatibility with makefile.
> > > > However, I doubt this value is used by any application.
> > > > I think we can try to remove RTE_MACHINE from meson builds in
> DPDK
> > > > 19.11, or use RTE_MACHINE as micro-arch (my preference).
> > > 'MACHINE' means different things to different people, which is the root
> > cause of this discussion.
> > > 'MICRO-ARCH' has a very clear meaning. Do you see any problem going
> > with MICRO-ARCH instead?
> >
> > Some applications may use RTE_MACHINE for this purpose.
> > It is part of the API since the befinning of DPDK.
> > I don't see a real motivation to break this API now.
> The suggestions are not clear to me. The original suggestion was to
> introduce RTE_MICRO_ARCH and contain all the micro-architecture related
> compiler flags in that.
> Now, the suggestion is to use RTE_MACHINE to contain micro-architecture
> related compiler flags. Will it contain NUMA, number of cores as well (as
> suggested earlier)? If yes, I do not see it changing anything.
> 
> I am not a meson expert. However, I looked at various meson.build files. I
> have few questions/concerns.
> 1) Are these suggestions are for all the platforms? IMO, these need to be the
> same across all the architectures.
> 2) I am looking at config/meson.build. Here RTE_MACHINE is defined to
> indicate Architecture for Arm (armv7-a, aarchxx) and micro-architecture for
> x86 (corei7?). Is the understanding correct? IMO, this should this have a
> common meaning across all the platforms?
> 3) If the changes are for all the platforms, is the risk high for 19.11 release?
> 4) The N1 config patch as such conforms to the current conventions. What is
> being asked here is an enhancement, is the understanding correct?
> 
We consider it is an enhancement to introduce the RTE_MICRO_ARCH and I understand it is a high risk and we did not agree here yet.
I will submit v3 to change the configuration name from neoverse-n1 to n1sdp, to address Honnappa's comment.
/Gavin
  

Patch

diff --git a/config/arm/meson.build b/config/arm/meson.build
index 979018e..995d321 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -63,6 +63,12 @@  flags_armada = [
 	['RTE_MAX_LCORE', 16]]
 
 flags_default_extra = []
+flags_neoversen1_extra = [
+	['RTE_MACHINE', '"neoversen1"'],
+	['RTE_MAX_NUMA_NODES', 1],
+	['RTE_MAX_LCORE', 4],
+	['RTE_EAL_NUMA_AWARE_HUGEPAGES', false],
+	['RTE_LIBRTE_VHOST_NUMA', false]]
 flags_thunderx_extra = [
 	['RTE_MACHINE', '"thunderx"'],
 	['RTE_USE_C11_MEM_MODEL', false]]
@@ -87,7 +93,8 @@  machine_args_generic = [
 	['0xd07', ['-mcpu=cortex-a57']],
 	['0xd08', ['-mcpu=cortex-a72']],
 	['0xd09', ['-mcpu=cortex-a73']],
-	['0xd0a', ['-mcpu=cortex-a75']]]
+	['0xd0a', ['-mcpu=cortex-a75']],
+	['0xd0c', ['-march=armv8.2-a+crc+crypto', '-mcpu=neoverse-n1'], flags_neoversen1_extra]]
 
 machine_args_cavium = [
 	['default', ['-march=armv8-a+crc+crypto','-mcpu=thunderx']],
diff --git a/config/defconfig_arm64-neoversen1-linux-gcc b/config/defconfig_arm64-neoversen1-linux-gcc
new file mode 120000
index 0000000..47c96a4
--- /dev/null
+++ b/config/defconfig_arm64-neoversen1-linux-gcc
@@ -0,0 +1 @@ 
+defconfig_arm64-neoversen1-linuxapp-gcc
\ No newline at end of file
diff --git a/config/defconfig_arm64-neoversen1-linuxapp-gcc b/config/defconfig_arm64-neoversen1-linuxapp-gcc
new file mode 100644
index 0000000..39b9e1f
--- /dev/null
+++ b/config/defconfig_arm64-neoversen1-linuxapp-gcc
@@ -0,0 +1,15 @@ 
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2019 Arm Ltd.
+#
+
+#include "defconfig_arm64-armv8a-linux-gcc"
+
+CONFIG_RTE_MACHINE="neoversen1"
+CONFIG_RTE_ARCH_ARM_TUNE="neoverse-n1"
+CONFIG_RTE_MAX_LCORE=4
+CONFIG_RTE_MAX_NUMA_NODES=1
+CONFIG_RTE_CACHE_LINE_SIZE=64
+
+# Doesn't support NUMA
+CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n
+CONFIG_RTE_LIBRTE_VHOST_NUMA=n
diff --git a/mk/machine/neoversen1/rte.vars.mk b/mk/machine/neoversen1/rte.vars.mk
new file mode 100644
index 0000000..6d69de0
--- /dev/null
+++ b/mk/machine/neoversen1/rte.vars.mk
@@ -0,0 +1,34 @@ 
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2019 Arm Ltd
+#
+
+#
+# machine:
+#
+#   - can define ARCH variable (overridden by cmdline value)
+#   - can define CROSS variable (overridden by cmdline value)
+#   - define MACHINE_CFLAGS variable (overridden by cmdline value)
+#   - define MACHINE_LDFLAGS variable (overridden by cmdline value)
+#   - define MACHINE_ASFLAGS variable (overridden by cmdline value)
+#   - can define CPU_CFLAGS variable (overridden by cmdline value) that
+#     overrides the one defined in arch.
+#   - can define CPU_LDFLAGS variable (overridden by cmdline value) that
+#     overrides the one defined in arch.
+#   - can define CPU_ASFLAGS variable (overridden by cmdline value) that
+#     overrides the one defined in arch.
+#   - may override any previously defined variable
+#
+
+# ARCH =
+# CROSS =
+# MACHINE_CFLAGS =
+# MACHINE_LDFLAGS =
+# MACHINE_ASFLAGS =
+# CPU_CFLAGS =
+# CPU_LDFLAGS =
+# CPU_ASFLAGS =
+
+include $(RTE_SDK)/mk/rte.helper.mk
+
+MACHINE_CFLAGS += $(call rte_cc_has_argument, -march=armv8.2-a+crc+crypto)
+MACHINE_CFLAGS += $(call rte_cc_has_argument, -mcpu=neoverse-n1)