Re: [dpdk-dev] [PATCH v2 2/3] config: add arm neoverse N1 SDP configuration

[Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>]

<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?

>