mbox series

[v14,00/12] Arm build options rework

Message ID 1608724059-8562-1-git-send-email-juraj.linkes@pantheon.tech (mailing list archive)
Headers
Series Arm build options rework |

Message

Juraj Linkeš Dec. 23, 2020, 11:47 a.m. UTC
  The current way of specifying Arm configuration options is insufficient
since we can't identify the SoC we're building for from the MIDR
information. For example, we can't distinguish between N1SDP, Graviton2
or Ampere Altra.

Add a way to specify the cpu count and numa node count for cross builds
and aarch64 -> aarch64 (SoC) builds.

We also want to be able to disable which drivers (and possibly
libraries) are built without user input. This is useful when building:
1. on an SoC that is slow and we want to build only what is necessary
  without the user having to check which libraries they have installed
2. a cross build on a fast aarch64 machine but with target SoC which
  differs in capabilities or libraries.
This is achieved by specifying the drivers in SoC configuration.

Among libraries, only libnuma can be now disabled.

Also add an optional way to discover cpu count a numa node count. Fix
-Dmax_lcores and -Dmax_numa_nodes for arm builds.

The current implementation adds/supports the following:
* x86 -> aarch64 cross build with added config options/disabled
  drivers/libs
* aarch64 -> aarch64 builds for a different SoCs using meson -Darm_soc
  option or using a cross file
* max numa nodes and max lcore may be specified on the command line to
  overwrite the values for any (native, SoC or cross) build

v2:
Major rework of the whole series.

v3:
Added numa and core count defaults for x86 default build.
Removed numa and core count defaults. Now requiring defaults to be
specified in a cross file or on the cmdline.
Added FreeBDS support for numa count discovery.

v4:
Make automatic numa and cpu counts discovery optional.

v5:
Split the refactor patch into smaller patches.
Simplify buildtools/get_numa_count.py.
Add more explanation to cover letter.

v6:
Apply cross file options arch agnostically, not just in Arm cross
builds.
Streamline Arm build setup and machine args: always use native args in
native builds, require implementer ID and part number for cross builds.

v7:
Arm config options are now organized in one dictionary.
Removed unsupported implementers and removed fallback to generic
implementer/part number for unknown implementer/part number.
Added meson config option arm_soc which can be used to specify
configuration to be used, useful for aarch64 -> aarch64 builds.

v8:
Rebase.

v9:
Split SoC and implementer dictionaries into smaller parts.
Fixed DPAA and ARMADA SoC configuration.
Added documentation about supported SoCs.

v10:
Added a commit that fixes Graviton2 clang build failures in native
build.

v11:
Rebase.

v12:
Fixed dpaa and armada cross builds in generic build commit.

v13:
Removed two commits which were not arm-specific:
1. build: alias default build as generic
2. build: optional NUMA and cpu counts detection
Also minor adjustments to commit messages in:
1. build: isolate configuration for Arm generic build
2. build: disable drivers in Arm builds

v14:
Resent for retesting (because of suspected false negative).

Series Acked-by: Jerin Jacob <jerinj@marvell.com>
Series Tested-by: Jerin Jacob <jerinj@marvell.com>
Series Tested-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Series Tested-by: Vimal Chungath <vcchunga@amazon.com>

Juraj Linkeš (12):
  build: rename Arm build variables
  build: remove unused or superfluous variables
  build: reformat and move Arm config and comments
  build: simplify how Arm flags are processed
  build: organize Arm config into dict
  build: isolate configuration for Arm generic build
  build: use native machine args in Arm native build
  build: add core and NUMA counts to cross files
  build: disable drivers in Arm builds
  build: disable libnuma in cross builds
  build: add Arm SoC meson option
  config: fix Arm implementer and its SoCs

 config/arm/arm64_armada_linux_gcc             |   2 +-
 config/arm/arm64_armv8_linux_gcc              |  15 +-
 config/arm/arm64_bluefield_linux_gcc          |   3 +-
 config/arm/arm64_dpaa_linux_gcc               |   2 +-
 config/arm/arm64_emag_linux_gcc               |   2 +-
 config/arm/arm64_graviton2_linux_gcc          |   3 +-
 config/arm/arm64_n1sdp_linux_gcc              |   3 +-
 config/arm/arm64_octeontx2_linux_gcc          |   3 +-
 config/arm/arm64_stingray_linux_gcc           |   3 +-
 config/arm/arm64_thunderx2_linux_gcc          |   3 +-
 ..._linux_gcc => arm64_thunderxt88_linux_gcc} |   2 +-
 config/arm/meson.build                        | 502 ++++++++++++------
 config/meson.build                            |  34 +-
 .../linux_gsg/cross_build_dpdk_for_arm64.rst  |  30 ++
 drivers/meson.build                           |   6 +-
 meson.build                                   |   1 +
 meson_options.txt                             |   2 +
 17 files changed, 413 insertions(+), 203 deletions(-)
 rename config/arm/{arm64_thunderx_linux_gcc => arm64_thunderxt88_linux_gcc} (92%)
  

Comments

Andrew Boyer Dec. 30, 2020, 7:09 p.m. UTC | #1
> On Dec 23, 2020, at 6:47 AM, Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> 
> The current way of specifying Arm configuration options is insufficient
> since we can't identify the SoC we're building for from the MIDR
> information. For example, we can't distinguish between N1SDP, Graviton2
> or Ampere Altra.
> 
> Add a way to specify the cpu count and numa node count for cross builds
> and aarch64 -> aarch64 (SoC) builds.
> 

Hello Juraj,
This is great, you have solved a problem for me before I even knew there was one. (We have two SoCs with the same id and pn, but different core counts etc.)

Can anyone fill me in on how and when this patchset is going to be taken? Will it go to dpdk-next-net, or to some other branch?

Thank you,
Andrew
  
Thomas Monjalon Dec. 30, 2020, 8:56 p.m. UTC | #2
30/12/2020 20:09, Andrew Boyer:
> 
> > On Dec 23, 2020, at 6:47 AM, Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> > 
> > The current way of specifying Arm configuration options is insufficient
> > since we can't identify the SoC we're building for from the MIDR
> > information. For example, we can't distinguish between N1SDP, Graviton2
> > or Ampere Altra.
> > 
> > Add a way to specify the cpu count and numa node count for cross builds
> > and aarch64 -> aarch64 (SoC) builds.
> > 
> 
> Hello Juraj,
> This is great, you have solved a problem for me before I even knew there was one. (We have two SoCs with the same id and pn, but different core counts etc.)
> 
> Can anyone fill me in on how and when this patchset is going to be taken? Will it go to dpdk-next-net, or to some other branch?

It should go in the main branch.
I cannot commit on any date, but for sure it would help
if you can do a detailed review, thanks.
  
Honnappa Nagarahalli Jan. 1, 2021, 5:19 p.m. UTC | #3
<snip>

> 
> 30/12/2020 20:09, Andrew Boyer:
> >
> > > On Dec 23, 2020, at 6:47 AM, Juraj Linkeš <juraj.linkes@pantheon.tech>
> wrote:
> > >
> > > The current way of specifying Arm configuration options is
> > > insufficient since we can't identify the SoC we're building for from
> > > the MIDR information. For example, we can't distinguish between
> > > N1SDP, Graviton2 or Ampere Altra.
> > >
> > > Add a way to specify the cpu count and numa node count for cross
> > > builds and aarch64 -> aarch64 (SoC) builds.
> > >
> >
> > Hello Juraj,
> > This is great, you have solved a problem for me before I even knew
> > there was one. (We have two SoCs with the same id and pn, but
> > different core counts etc.)
> >
> > Can anyone fill me in on how and when this patchset is going to be taken?
> Will it go to dpdk-next-net, or to some other branch?
> 
> It should go in the main branch.
> I cannot commit on any date, but for sure it would help if you can do a
> detailed review, thanks.
Testing on your SoC would be of great help.


>
  
Andrew Boyer Jan. 4, 2021, 10:46 p.m. UTC | #4
> On Jan 1, 2021, at 12:19 PM, Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
>> 
>> 30/12/2020 20:09, Andrew Boyer:
>>> 
>>>> On Dec 23, 2020, at 6:47 AM, Juraj Linkeš <juraj.linkes@pantheon.tech>
>> wrote:
>>>> 
>>>> The current way of specifying Arm configuration options is
>>>> insufficient since we can't identify the SoC we're building for from
>>>> the MIDR information. For example, we can't distinguish between
>>>> N1SDP, Graviton2 or Ampere Altra.
>>>> 
>>>> Add a way to specify the cpu count and numa node count for cross
>>>> builds and aarch64 -> aarch64 (SoC) builds.
>>>> 
>>> 
>>> Hello Juraj,
>>> This is great, you have solved a problem for me before I even knew
>>> there was one. (We have two SoCs with the same id and pn, but
>>> different core counts etc.)
>>> 
>>> Can anyone fill me in on how and when this patchset is going to be taken?
>> Will it go to dpdk-next-net, or to some other branch?
>> 
>> It should go in the main branch.
>> I cannot commit on any date, but for sure it would help if you can do a
>> detailed review, thanks.
> Testing on your SoC would be of great help.
> 

Hello Honnappa, Juraj, and Bruce,

I've got most of the build working under meson. A few questions:

1)  Bruce - when the “-Ddefault_library=both” flag is passed in, the build fails with this error. It’s been broken for a long time; maybe this option isn’t supported and should be blocked earlier?

	../../dpdk/app/meson.build:48:3: ERROR: Tried to get unknown variable "both_rte_ethdev".

2) Is there a way to disable specific libraries? I’ve pruned down the list of drivers, which is great. This feature existed under make but I don’t see anything about it in meson yet.

3) We need to build kni against the aarch64 kernel headers, but it fails. It appears that kernel/linux/kni/meson.build doesn’t pass any cross-compile flags in the make command it creates. The diff below shows how I hardcoded it to get it to work for now. Thoughts on how to do this right? meson has the path (in $PATH) and the binary prefixs (in ‘[binaries]’ in the cross file). It must know the arch, too.

--- a/kernel/linux/kni/meson.build
+++ b/kernel/linux/kni/meson.build
@@ -14,6 +14,8 @@ custom_target('rte_kni',
        input: kni_sources,
        output: 'rte_kni.ko',
        command: ['make', '-j4', '-C', kernel_dir + '/build',
+               'CROSS_COMPILE=/tool/toolchain/aarch64-1.1/bin/aarch64-linux-gnu-',
+               'ARCH=aarch64',
                'M=' + meson.current_build_dir(),
                'src=' + meson.current_source_dir(),
                'MODULE_CFLAGS=-include ' + meson.source_root() + '/config/rte_config.h' +

I will try to get to a full review soon.

Thank you,
Andrew
  
Pavan Nikhilesh Bhagavatula Jan. 5, 2021, 11:02 a.m. UTC | #5
>-----Original Message-----
>From: dev <dev-bounces@dpdk.org> On Behalf Of Juraj Linkeš
>Sent: Wednesday, December 23, 2020 5:17 PM
>To: bruce.richardson@intel.com; Ruifeng.Wang@arm.com;
>Honnappa.Nagarahalli@arm.com; Phil.Yang@arm.com;
>vcchunga@amazon.com; Dharmik.Thakkar@arm.com;
>jerinjacobk@gmail.com; hemant.agrawal@nxp.com;
>ajit.khaparde@broadcom.com; ferruh.yigit@intel.com
>Cc: dev@dpdk.org; Juraj Linkeš <juraj.linkes@pantheon.tech>
>Subject: [dpdk-dev] [PATCH v14 00/12] Arm build options rework
>
>The current way of specifying Arm configuration options is insufficient
>since we can't identify the SoC we're building for from the MIDR
>information. For example, we can't distinguish between N1SDP,
>Graviton2
>or Ampere Altra.
>
>Add a way to specify the cpu count and numa node count for cross
>builds
>and aarch64 -> aarch64 (SoC) builds.
>
>We also want to be able to disable which drivers (and possibly
>libraries) are built without user input. This is useful when building:
>1. on an SoC that is slow and we want to build only what is necessary
>  without the user having to check which libraries they have installed
>2. a cross build on a fast aarch64 machine but with target SoC which
>  differs in capabilities or libraries.
>This is achieved by specifying the drivers in SoC configuration.
>
>Among libraries, only libnuma can be now disabled.
>
>Also add an optional way to discover cpu count a numa node count. Fix
>-Dmax_lcores and -Dmax_numa_nodes for arm builds.
>
>The current implementation adds/supports the following:
>* x86 -> aarch64 cross build with added config options/disabled
>  drivers/libs
>* aarch64 -> aarch64 builds for a different SoCs using meson -Darm_soc
>  option or using a cross file
>* max numa nodes and max lcore may be specified on the command
>line to
>  overwrite the values for any (native, SoC or cross) build
>
>v2:
>Major rework of the whole series.
>
>v3:
>Added numa and core count defaults for x86 default build.
>Removed numa and core count defaults. Now requiring defaults to be
>specified in a cross file or on the cmdline.
>Added FreeBDS support for numa count discovery.
>
>v4:
>Make automatic numa and cpu counts discovery optional.
>
>v5:
>Split the refactor patch into smaller patches.
>Simplify buildtools/get_numa_count.py.
>Add more explanation to cover letter.
>
>v6:
>Apply cross file options arch agnostically, not just in Arm cross
>builds.
>Streamline Arm build setup and machine args: always use native args in
>native builds, require implementer ID and part number for cross builds.
>
>v7:
>Arm config options are now organized in one dictionary.
>Removed unsupported implementers and removed fallback to generic
>implementer/part number for unknown implementer/part number.
>Added meson config option arm_soc which can be used to specify
>configuration to be used, useful for aarch64 -> aarch64 builds.
>
>v8:
>Rebase.
>
>v9:
>Split SoC and implementer dictionaries into smaller parts.
>Fixed DPAA and ARMADA SoC configuration.
>Added documentation about supported SoCs.
>
>v10:
>Added a commit that fixes Graviton2 clang build failures in native
>build.
>
>v11:
>Rebase.
>
>v12:
>Fixed dpaa and armada cross builds in generic build commit.
>
>v13:
>Removed two commits which were not arm-specific:
>1. build: alias default build as generic
>2. build: optional NUMA and cpu counts detection
>Also minor adjustments to commit messages in:
>1. build: isolate configuration for Arm generic build
>2. build: disable drivers in Arm builds
>
>v14:
>Resent for retesting (because of suspected false negative).
>
>Series Acked-by: Jerin Jacob <jerinj@marvell.com>
>Series Tested-by: Jerin Jacob <jerinj@marvell.com>
>Series Tested-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>Series Tested-by: Vimal Chungath <vcchunga@amazon.com>
>

Series Tested-by: Pavan Nikhilesh <pbhagavatula@marvell.com>

Also, I believe we need to take a look at ARCH_SOC_ID support 
coming up in 5.9 kernel.

https://lore.kernel.org/linux-arm-kernel/20200506164411.3284-1-sudeep.holla@arm.com/

>Juraj Linkeš (12):
>  build: rename Arm build variables
>  build: remove unused or superfluous variables
>  build: reformat and move Arm config and comments
>  build: simplify how Arm flags are processed
>  build: organize Arm config into dict
>  build: isolate configuration for Arm generic build
>  build: use native machine args in Arm native build
>  build: add core and NUMA counts to cross files
>  build: disable drivers in Arm builds
>  build: disable libnuma in cross builds
>  build: add Arm SoC meson option
>  config: fix Arm implementer and its SoCs
>
> config/arm/arm64_armada_linux_gcc             |   2 +-
> config/arm/arm64_armv8_linux_gcc              |  15 +-
> config/arm/arm64_bluefield_linux_gcc          |   3 +-
> config/arm/arm64_dpaa_linux_gcc               |   2 +-
> config/arm/arm64_emag_linux_gcc               |   2 +-
> config/arm/arm64_graviton2_linux_gcc          |   3 +-
> config/arm/arm64_n1sdp_linux_gcc              |   3 +-
> config/arm/arm64_octeontx2_linux_gcc          |   3 +-
> config/arm/arm64_stingray_linux_gcc           |   3 +-
> config/arm/arm64_thunderx2_linux_gcc          |   3 +-
> ..._linux_gcc => arm64_thunderxt88_linux_gcc} |   2 +-
> config/arm/meson.build                        | 502 ++++++++++++------
> config/meson.build                            |  34 +-
> .../linux_gsg/cross_build_dpdk_for_arm64.rst  |  30 ++
> drivers/meson.build                           |   6 +-
> meson.build                                   |   1 +
> meson_options.txt                             |   2 +
> 17 files changed, 413 insertions(+), 203 deletions(-)
> rename config/arm/{arm64_thunderx_linux_gcc =>
>arm64_thunderxt88_linux_gcc} (92%)
>
>--
>2.20.1
  
Honnappa Nagarahalli Jan. 5, 2021, 3:12 p.m. UTC | #6
<snip>

> >
> >v14:
> >Resent for retesting (because of suspected false negative).
> >
> >Series Acked-by: Jerin Jacob <jerinj@marvell.com> Series Tested-by:
> >Jerin Jacob <jerinj@marvell.com> Series Tested-by: Dharmik Thakkar
> ><dharmik.thakkar@arm.com> Series Tested-by: Vimal Chungath
> ><vcchunga@amazon.com>
> >
> 
> Series Tested-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> 
> Also, I believe we need to take a look at ARCH_SOC_ID support coming up in
> 5.9 kernel.
> 
> https://lore.kernel.org/linux-arm-kernel/20200506164411.3284-1-
> sudeep.holla@arm.com/

Agree, it can come after this series is merged.

Note that it requires firmware with SMCCC v1.2 support in the underlying platform.

> 
> >Juraj Linkeš (12):
> >  build: rename Arm build variables
> >  build: remove unused or superfluous variables
> >  build: reformat and move Arm config and comments
> >  build: simplify how Arm flags are processed
> >  build: organize Arm config into dict
> >  build: isolate configuration for Arm generic build
> >  build: use native machine args in Arm native build
> >  build: add core and NUMA counts to cross files
> >  build: disable drivers in Arm builds
> >  build: disable libnuma in cross builds
> >  build: add Arm SoC meson option
> >  config: fix Arm implementer and its SoCs
> >
> > config/arm/arm64_armada_linux_gcc             |   2 +-
> > config/arm/arm64_armv8_linux_gcc              |  15 +-
> > config/arm/arm64_bluefield_linux_gcc          |   3 +-
> > config/arm/arm64_dpaa_linux_gcc               |   2 +-
> > config/arm/arm64_emag_linux_gcc               |   2 +-
> > config/arm/arm64_graviton2_linux_gcc          |   3 +-
> > config/arm/arm64_n1sdp_linux_gcc              |   3 +-
> > config/arm/arm64_octeontx2_linux_gcc          |   3 +-
> > config/arm/arm64_stingray_linux_gcc           |   3 +-
> > config/arm/arm64_thunderx2_linux_gcc          |   3 +-
> > ..._linux_gcc => arm64_thunderxt88_linux_gcc} |   2 +-
> > config/arm/meson.build                        | 502 ++++++++++++------
> > config/meson.build                            |  34 +-
> > .../linux_gsg/cross_build_dpdk_for_arm64.rst  |  30 ++
> > drivers/meson.build                           |   6 +-
> > meson.build                                   |   1 +
> > meson_options.txt                             |   2 +
> > 17 files changed, 413 insertions(+), 203 deletions(-)  rename
> >config/arm/{arm64_thunderx_linux_gcc => arm64_thunderxt88_linux_gcc}
> >(92%)
> >
> >--
> >2.20.1
  
Bruce Richardson Jan. 6, 2021, 1:40 p.m. UTC | #7
On Mon, Jan 04, 2021 at 05:46:20PM -0500, Andrew Boyer wrote:
> 
> > On Jan 1, 2021, at 12:19 PM, Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
> >> 
> >> 30/12/2020 20:09, Andrew Boyer:
> >>> 
> >>>> On Dec 23, 2020, at 6:47 AM, Juraj Linkeš <juraj.linkes@pantheon.tech>
> >> wrote:
> >>>> 
> >>>> The current way of specifying Arm configuration options is
> >>>> insufficient since we can't identify the SoC we're building for from
> >>>> the MIDR information. For example, we can't distinguish between
> >>>> N1SDP, Graviton2 or Ampere Altra.
> >>>> 
> >>>> Add a way to specify the cpu count and numa node count for cross
> >>>> builds and aarch64 -> aarch64 (SoC) builds.
> >>>> 
> >>> 
> >>> Hello Juraj,
> >>> This is great, you have solved a problem for me before I even knew
> >>> there was one. (We have two SoCs with the same id and pn, but
> >>> different core counts etc.)
> >>> 
> >>> Can anyone fill me in on how and when this patchset is going to be taken?
> >> Will it go to dpdk-next-net, or to some other branch?
> >> 
> >> It should go in the main branch.
> >> I cannot commit on any date, but for sure it would help if you can do a
> >> detailed review, thanks.
> > Testing on your SoC would be of great help.
> > 
> 
> Hello Honnappa, Juraj, and Bruce,
> 
> I've got most of the build working under meson. A few questions:
> 
> 1)  Bruce - when the “-Ddefault_library=both” flag is passed in, the build fails with this error. It’s been broken for a long time; maybe this option isn’t supported and should be blocked earlier?
> 

Yes, we should catch this earlier.

> 	../../dpdk/app/meson.build:48:3: ERROR: Tried to get unknown variable "both_rte_ethdev".
> 
> 2) Is there a way to disable specific libraries? I’ve pruned down the list of drivers, which is great. This feature existed under make but I don’t see anything about it in meson yet.
> 
> 3) We need to build kni against the aarch64 kernel headers, but it fails. It appears that kernel/linux/kni/meson.build doesn’t pass any cross-compile flags in the make command it creates. The diff below shows how I hardcoded it to get it to work for now. Thoughts on how to do this right? meson has the path (in $PATH) and the binary prefixs (in ‘[binaries]’ in the cross file). It must know the arch, too.
> 

I'll try and investigate a little on these. Patches welcome, especially for
issue #3 which is new to me.
For #2, there has been some previous discussion about this on list before.

/Bruce

> --- a/kernel/linux/kni/meson.build
> +++ b/kernel/linux/kni/meson.build
> @@ -14,6 +14,8 @@ custom_target('rte_kni',
>         input: kni_sources,
>         output: 'rte_kni.ko',
>         command: ['make', '-j4', '-C', kernel_dir + '/build',
> +               'CROSS_COMPILE=/tool/toolchain/aarch64-1.1/bin/aarch64-linux-gnu-',
> +               'ARCH=aarch64',
>                 'M=' + meson.current_build_dir(),
>                 'src=' + meson.current_source_dir(),
>                 'MODULE_CFLAGS=-include ' + meson.source_root() + '/config/rte_config.h' +
> 
> I will try to get to a full review soon.
> 
> Thank you,
> Andrew
  
Bruce Richardson Jan. 8, 2021, 5:36 p.m. UTC | #8
On Mon, Jan 04, 2021 at 05:46:20PM -0500, Andrew Boyer wrote:
> 
> 1)  Bruce - when the “-Ddefault_library=both” flag is passed in, the build fails with this error. It’s been broken for a long time; maybe this option isn’t supported and should be blocked earlier?
> 
> 	../../dpdk/app/meson.build:48:3: ERROR: Tried to get unknown variable "both_rte_ethdev".
> 
Revisiting this point, since there are a number of possible approaches we
can take here, and I'd like feedback on them before we do anything. Of
these approaches, 2 are simple, and 1 is more complicated.

1. We can just detect this as an invalid/unsupported setting and error out
earlier with a suitable errors message
2. Since we already support in all cases building "both" libraries, we can
just convert "both" to be the same as "shared" for app linking (since app
linking is all the value is actually used for by us)
3. We can change how we do the builds to actually repect the value as
intended - only build static or shared libs as requested and only build
both when value "both" is passed.

Of these, #3 is obviously the most work, but may make the most sense to
users. However, it does have the following open issues - which are all linked:
1. What way do we make the default linkage in the case of "both" being
selected?
2. What do we make the default for builds?
3. Is there ever a case where someone needs both libraries build but
non-default linkage?

The initial answers I thought for these would be that the default for
"both" builds should be "shared", and that "both" should be the default.
However, that then is a behaviour change for DPDK since we don't get
static binaries by default. The solution to that is that we change the
default to "static", but then we revert to the situation we used to have
with the make build where we regularly got patches upstreamed which failed
to build with shared libs because the map file update was missing and the
submitter never tested shared builds. The third alternative there is that
we default to "both" but have "static" binaries when that is selected. That
option runs against question #3 where I suspect there will be cases (e.g.
packaging) where one does want both library sets generated but shared
binaries.

Therefore, is keeping things largely as they are and taking the simpler
options #1 or #2 better? Do we want to look into #3 but with further
additions such as a "default_app_linkage" DPDK option, or linking all apps
twice when "both" is specified as "default_library"?

Feedback welcome please.

/Bruce

PS: For background, when the DPDK transition to meson started, there
wasn't a "both" option which is why this is not the best supported. :-)
  
Honnappa Nagarahalli Jan. 8, 2021, 8:20 p.m. UTC | #9
<snip>

> 
> On Mon, Jan 04, 2021 at 05:46:20PM -0500, Andrew Boyer wrote:
> >
> > 1)  Bruce - when the “-Ddefault_library=both” flag is passed in, the build
> fails with this error. It’s been broken for a long time; maybe this option isn’t
> supported and should be blocked earlier?
> >
> > 	../../dpdk/app/meson.build:48:3: ERROR: Tried to get unknown
> variable "both_rte_ethdev".
> >
> Revisiting this point, since there are a number of possible approaches we can
> take here, and I'd like feedback on them before we do anything. Of these
> approaches, 2 are simple, and 1 is more complicated.
> 
> 1. We can just detect this as an invalid/unsupported setting and error out
> earlier with a suitable errors message 2. Since we already support in all cases
I would prefer option 1 here (detect and error out). IMO, the option "both" does not seem to solve a compelling problem. I would prefer to avoid the additional code and complications. Mostly, everyone would do the development with either 'static' or 'shared' and test the other at the end when the development is completed.

> building "both" libraries, we can just convert "both" to be the same as
> "shared" for app linking (since app linking is all the value is actually used for
I do not think option 2 solves the problem completely. i.e. if the user specifies 'both', app needs to be built for both static and shared libraries.

> by us) 3. We can change how we do the builds to actually repect the value as
> intended - only build static or shared libs as requested and only build both
> when value "both" is passed.
> 
> Of these, #3 is obviously the most work, but may make the most sense to
> users. However, it does have the following open issues - which are all linked:
> 1. What way do we make the default linkage in the case of "both" being
> selected?
> 2. What do we make the default for builds?
> 3. Is there ever a case where someone needs both libraries build but non-
> default linkage?
> 
> The initial answers I thought for these would be that the default for "both"
> builds should be "shared", and that "both" should be the default.
> However, that then is a behaviour change for DPDK since we don't get static
> binaries by default. The solution to that is that we change the default to
> "static", but then we revert to the situation we used to have with the make
> build where we regularly got patches upstreamed which failed to build with
> shared libs because the map file update was missing and the submitter never
> tested shared builds.
I have been one among submitting the patches without testing for shared libraries. But, I am thinking that we have a good CI now that compiles for various combinations. I think that should be enough.

The third alternative there is that we default to "both"
> but have "static" binaries when that is selected. That option runs against
> question #3 where I suspect there will be cases (e.g.
> packaging) where one does want both library sets generated but shared
> binaries.
> 
> Therefore, is keeping things largely as they are and taking the simpler options
> #1 or #2 better? Do we want to look into #3 but with further additions such
> as a "default_app_linkage" DPDK option, or linking all apps twice when
> "both" is specified as "default_library"?
I would say stick with #1 as there is not a compelling problem to solve.

> 
> Feedback welcome please.
> 
> /Bruce
> 
> PS: For background, when the DPDK transition to meson started, there
> wasn't a "both" option which is why this is not the best supported. :-)
  
Thomas Monjalon Jan. 11, 2021, 9:38 a.m. UTC | #10
08/01/2021 21:20, Honnappa Nagarahalli:
> > On Mon, Jan 04, 2021 at 05:46:20PM -0500, Andrew Boyer wrote:
> > >
> > > 1)  Bruce - when the “-Ddefault_library=both” flag is passed in, the build
> > fails with this error. It’s been broken for a long time; maybe this option isn’t
> > supported and should be blocked earlier?
> > >
> > > 	../../dpdk/app/meson.build:48:3: ERROR: Tried to get unknown
> > variable "both_rte_ethdev".
> > >
> > Revisiting this point, since there are a number of possible approaches we can
> > take here, and I'd like feedback on them before we do anything. Of these
> > approaches, 2 are simple, and 1 is more complicated.
> > 
> > 1. We can just detect this as an invalid/unsupported setting and error out
> > earlier with a suitable errors message 2. Since we already support in all cases
> 
> I would prefer option 1 here (detect and error out). IMO, the option "both" does not seem to solve a compelling problem. I would prefer to avoid the additional code and complications. Mostly, everyone would do the development with either 'static' or 'shared' and test the other at the end when the development is completed.

+1 for not supporting linking with both.


> > building "both" libraries, we can just convert "both" to be the same as
> > "shared" for app linking (since app linking is all the value is actually used for
> I do not think option 2 solves the problem completely. i.e. if the user specifies 'both', app needs to be built for both static and shared libraries.
> 
> > by us) 3. We can change how we do the builds to actually repect the value as
> > intended - only build static or shared libs as requested and only build both
> > when value "both" is passed.
> > 
> > Of these, #3 is obviously the most work, but may make the most sense to
> > users. However, it does have the following open issues - which are all linked:
> > 1. What way do we make the default linkage in the case of "both" being
> > selected?
> > 2. What do we make the default for builds?
> > 3. Is there ever a case where someone needs both libraries build but non-
> > default linkage?
> > 
> > The initial answers I thought for these would be that the default for "both"
> > builds should be "shared", and that "both" should be the default.
> > However, that then is a behaviour change for DPDK since we don't get static
> > binaries by default. The solution to that is that we change the default to
> > "static", but then we revert to the situation we used to have with the make
> > build where we regularly got patches upstreamed which failed to build with
> > shared libs because the map file update was missing and the submitter never
> > tested shared builds.
> 
> I have been one among submitting the patches without testing for shared libraries. But, I am thinking that we have a good CI now that compiles for various combinations. I think that should be enough.
> 
> > The third alternative there is that we default to "both"
> > but have "static" binaries when that is selected. That option runs against
> > question #3 where I suspect there will be cases (e.g.
> > packaging) where one does want both library sets generated but shared
> > binaries.
> > 
> > Therefore, is keeping things largely as they are and taking the simpler options
> > #1 or #2 better? Do we want to look into #3 but with further additions such
> > as a "default_app_linkage" DPDK option, or linking all apps twice when
> > "both" is specified as "default_library"?
> 
> I would say stick with #1 as there is not a compelling problem to solve.

me too
And I want to avoid linking static apps by default because they are huge.
  
Bruce Richardson Jan. 11, 2021, 10:01 a.m. UTC | #11
On Mon, Jan 11, 2021 at 10:38:09AM +0100, Thomas Monjalon wrote:
> 08/01/2021 21:20, Honnappa Nagarahalli:
> > > On Mon, Jan 04, 2021 at 05:46:20PM -0500, Andrew Boyer wrote:
> > > >
> > > > 1)  Bruce - when the “-Ddefault_library=both” flag is passed in, the build
> > > fails with this error. It’s been broken for a long time; maybe this option isn’t
> > > supported and should be blocked earlier?
> > > >
> > > > 	../../dpdk/app/meson.build:48:3: ERROR: Tried to get unknown
> > > variable "both_rte_ethdev".
> > > >
> > > Revisiting this point, since there are a number of possible approaches we can
> > > take here, and I'd like feedback on them before we do anything. Of these
> > > approaches, 2 are simple, and 1 is more complicated.
> > > 
> > > 1. We can just detect this as an invalid/unsupported setting and error out
> > > earlier with a suitable errors message 2. Since we already support in all cases
> > 
> > I would prefer option 1 here (detect and error out). IMO, the option "both" does not seem to solve a compelling problem. I would prefer to avoid the additional code and complications. Mostly, everyone would do the development with either 'static' or 'shared' and test the other at the end when the development is completed.
> 
> +1 for not supporting linking with both.
> 
Ok, thanks for the clear consensus. Will do patch to check and error out
appropriately.

/Bruce
  
Andrew Boyer Jan. 11, 2021, 4:16 p.m. UTC | #12
> On Jan 11, 2021, at 5:01 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> On Mon, Jan 11, 2021 at 10:38:09AM +0100, Thomas Monjalon wrote:
>> 08/01/2021 21:20, Honnappa Nagarahalli:
>>>> On Mon, Jan 04, 2021 at 05:46:20PM -0500, Andrew Boyer wrote:
>>>>> 
>>>>> 1)  Bruce - when the “-Ddefault_library=both” flag is passed in, the build
>>>> fails with this error. It’s been broken for a long time; maybe this option isn’t
>>>> supported and should be blocked earlier?
>>>>> 
>>>>> 	../../dpdk/app/meson.build:48:3: ERROR: Tried to get unknown
>>>> variable "both_rte_ethdev".
>>>>> 
>>>> Revisiting this point, since there are a number of possible approaches we can
>>>> take here, and I'd like feedback on them before we do anything. Of these
>>>> approaches, 2 are simple, and 1 is more complicated.
>>>> 
>>>> 1. We can just detect this as an invalid/unsupported setting and error out
>>>> earlier with a suitable errors message 2. Since we already support in all cases
>>> 
>>> I would prefer option 1 here (detect and error out). IMO, the option "both" does not seem to solve a compelling problem. I would prefer to avoid the additional code and complications. Mostly, everyone would do the development with either 'static' or 'shared' and test the other at the end when the development is completed.
>> 
>> +1 for not supporting linking with both.
>> 
> Ok, thanks for the clear consensus. Will do patch to check and error out
> appropriately.
> 
> /Bruce

It would be great if the error message for “both” pointed out that both sets of libraries are built in the “shared” case.
e.g.
‘Error: ‘both’ unsupported. ‘shared’ will build both static and shared libraries and dynamically-linked binaries. ‘static’ will build only static libraries and statically-linked binaries.’

-Andrew
  
Bruce Richardson Jan. 11, 2021, 4:59 p.m. UTC | #13
On Mon, Jan 11, 2021 at 11:16:38AM -0500, Andrew Boyer wrote:
>    On Jan 11, 2021, at 5:01 AM, Bruce Richardson
>    <[1]bruce.richardson@intel.com> wrote:
> 
>    On Mon, Jan 11, 2021 at 10:38:09AM +0100, Thomas Monjalon wrote:
> 
>      08/01/2021 21:20, Honnappa Nagarahalli:
> 
>      On Mon, Jan 04, 2021 at 05:46:20PM -0500, Andrew Boyer wrote:
> 
>      1)  Bruce - when the “-Ddefault_library=both” flag is passed in, the
>      build
> 
>      fails with this error. It’s been broken for a long time; maybe this
>      option isn’t
>      supported and should be blocked earlier?
> 
>      ../../dpdk/app/meson.build:48:3: ERROR: Tried to get unknown
> 
>      variable "both_rte_ethdev".
> 
>      Revisiting this point, since there are a number of possible
>      approaches we can
>      take here, and I'd like feedback on them before we do anything. Of
>      these
>      approaches, 2 are simple, and 1 is more complicated.
>      1. We can just detect this as an invalid/unsupported setting and
>      error out
>      earlier with a suitable errors message 2. Since we already support
>      in all cases
> 
>      I would prefer option 1 here (detect and error out). IMO, the option
>      "both" does not seem to solve a compelling problem. I would prefer
>      to avoid the additional code and complications. Mostly, everyone
>      would do the development with either 'static' or 'shared' and test
>      the other at the end when the development is completed.
> 
>      +1 for not supporting linking with both.
> 
>    Ok, thanks for the clear consensus. Will do patch to check and error
>    out
>    appropriately.
>    /Bruce
> 
>    It would be great if the error message for “both” pointed out that both
>    sets of libraries are built in the “shared” case.
> 
>    e.g.
> 
>    ‘Error: ‘both’ unsupported. ‘shared’ will build both static and shared
>    libraries and dynamically-linked binaries. ‘static’ will build only
>    static libraries and statically-linked binaries.’
> 
Actually "static" builds both libraries also, so default_library only
affects the linking of apps/examples in the DPDK build itself. I've put
that info in the patch error message as you suggested.

/Bruce
  
Juraj Linkeš Jan. 12, 2021, 8:28 a.m. UTC | #14
> -----Original Message-----
> From: Andrew Boyer <aboyer@pensando.io>
> Sent: Monday, January 4, 2021 11:46 PM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Cc: thomas@monjalon.net; Juraj Linkeš <juraj.linkes@pantheon.tech>;
> bruce.richardson@intel.com; Ruifeng Wang <Ruifeng.Wang@arm.com>; Phil
> Yang <Phil.Yang@arm.com>; vcchunga@amazon.com; Dharmik Thakkar
> <Dharmik.Thakkar@arm.com>; jerinjacobk@gmail.com;
> hemant.agrawal@nxp.com; Ajit Khaparde (ajit.khaparde@broadcom.com)
> <ajit.khaparde@broadcom.com>; ferruh.yigit@intel.com; dev@dpdk.org; nd
> <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v14 00/12] Arm build options rework
> 
> 
> > On Jan 1, 2021, at 12:19 PM, Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com> wrote:
> >>
> >> 30/12/2020 20:09, Andrew Boyer:
> >>>
> >>>> On Dec 23, 2020, at 6:47 AM, Juraj Linkeš
> >>>> <juraj.linkes@pantheon.tech>
> >> wrote:
> >>>>
> >>>> The current way of specifying Arm configuration options is
> >>>> insufficient since we can't identify the SoC we're building for
> >>>> from the MIDR information. For example, we can't distinguish
> >>>> between N1SDP, Graviton2 or Ampere Altra.
> >>>>
> >>>> Add a way to specify the cpu count and numa node count for cross
> >>>> builds and aarch64 -> aarch64 (SoC) builds.
> >>>>
> >>>
> >>> Hello Juraj,
> >>> This is great, you have solved a problem for me before I even knew
> >>> there was one. (We have two SoCs with the same id and pn, but
> >>> different core counts etc.)
> >>>
> >>> Can anyone fill me in on how and when this patchset is going to be taken?
> >> Will it go to dpdk-next-net, or to some other branch?
> >>
> >> It should go in the main branch.
> >> I cannot commit on any date, but for sure it would help if you can do
> >> a detailed review, thanks.
> > Testing on your SoC would be of great help.
> >
> 
> Hello Honnappa, Juraj, and Bruce,
> 

Let me add my 2 cents.

> I've got most of the build working under meson. A few questions:
> 
> 1)  Bruce - when the “-Ddefault_library=both” flag is passed in, the build fails
> with this error. It’s been broken for a long time; maybe this option isn’t
> supported and should be blocked earlier?
> 
> 	../../dpdk/app/meson.build:48:3: ERROR: Tried to get unknown variable
> "both_rte_ethdev".
> 

This doesn't seem like an arm specific problem. I understand Bruce is already working on a patch, so won't do anything with this in this series.

> 2) Is there a way to disable specific libraries? I’ve pruned down the list of drivers,
> which is great. This feature existed under make but I don’t see anything about it
> in meson yet.
> 

We're missing an arch agnostic way to disable these, so there's no support for it in this series. When that exists we can use that to disable libs for arm socs.

There was some discussion about this in http://patches.dpdk.org/patch/78077/, but maybe there's more.

> 3) We need to build kni against the aarch64 kernel headers, but it fails. It
> appears that kernel/linux/kni/meson.build doesn’t pass any cross-compile flags
> in the make command it creates. The diff below shows how I hardcoded it to get
> it to work for now. Thoughts on how to do this right? meson has the path (in
> $PATH) and the binary prefixs (in ‘[binaries]’ in the cross file). It must know the
> arch, too.
> 
> --- a/kernel/linux/kni/meson.build
> +++ b/kernel/linux/kni/meson.build
> @@ -14,6 +14,8 @@ custom_target('rte_kni',
>         input: kni_sources,
>         output: 'rte_kni.ko',
>         command: ['make', '-j4', '-C', kernel_dir + '/build',
> +               'CROSS_COMPILE=/tool/toolchain/aarch64-1.1/bin/aarch64-linux-
> gnu-',
> +               'ARCH=aarch64',
>                 'M=' + meson.current_build_dir(),
>                 'src=' + meson.current_source_dir(),
>                 'MODULE_CFLAGS=-include ' + meson.source_root() +
> '/config/rte_config.h' +
> 

Yes, Meson should know these. I'll submit this separately (in a new series), as this series is getting pretty big and this fix is actually unrelated.

> I will try to get to a full review soon.
> 
> Thank you,
> Andrew
  
Juraj Linkeš Jan. 22, 2021, 8:53 a.m. UTC | #15
> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Tuesday, January 5, 2021 4:12 PM
> To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Juraj Linkeš
> <juraj.linkes@pantheon.tech>; bruce.richardson@intel.com; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; Phil Yang <Phil.Yang@arm.com>;
> vcchunga@amazon.com; Dharmik Thakkar <Dharmik.Thakkar@arm.com>;
> jerinjacobk@gmail.com; hemant.agrawal@nxp.com; Ajit Khaparde
> (ajit.khaparde@broadcom.com) <ajit.khaparde@broadcom.com>;
> ferruh.yigit@intel.com
> Cc: dev@dpdk.org; nd <nd@arm.com>; nd <nd@arm.com>
> Subject: RE: [dpdk-dev] [PATCH v14 00/12] Arm build options rework
> 
> <snip>
> 
> > >
> > >v14:
> > >Resent for retesting (because of suspected false negative).
> > >
> > >Series Acked-by: Jerin Jacob <jerinj@marvell.com> Series Tested-by:
> > >Jerin Jacob <jerinj@marvell.com> Series Tested-by: Dharmik Thakkar
> > ><dharmik.thakkar@arm.com> Series Tested-by: Vimal Chungath
> > ><vcchunga@amazon.com>
> > >
> >
> > Series Tested-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >
> > Also, I believe we need to take a look at ARCH_SOC_ID support coming
> > up in
> > 5.9 kernel.
> >
> > https://lore.kernel.org/linux-arm-kernel/20200506164411.3284-1-
> > sudeep.holla@arm.com/
> 
> Agree, it can come after this series is merged.
> 
> Note that it requires firmware with SMCCC v1.2 support in the underlying
> platform.
> 

I have a few questions to everyone:
Is there any comprehensive documentation about how to use this? Things like where in Linux can I get the ARCH_SOC_ID and how are the ID's mapped to socs?
If the ID mappings are not public then the plaform owners would need to add the support, right?
  
Honnappa Nagarahalli Jan. 27, 2021, 1:40 p.m. UTC | #16
<snip>

> >
> > > >
> > > >v14:
> > > >Resent for retesting (because of suspected false negative).
> > > >
> > > >Series Acked-by: Jerin Jacob <jerinj@marvell.com> Series Tested-by:
> > > >Jerin Jacob <jerinj@marvell.com> Series Tested-by: Dharmik Thakkar
> > > ><dharmik.thakkar@arm.com> Series Tested-by: Vimal Chungath
> > > ><vcchunga@amazon.com>
> > > >
> > >
> > > Series Tested-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > >
> > > Also, I believe we need to take a look at ARCH_SOC_ID support coming
> > > up in
> > > 5.9 kernel.
> > >
> > > https://lore.kernel.org/linux-arm-kernel/20200506164411.3284-1-
> > > sudeep.holla@arm.com/
> >
> > Agree, it can come after this series is merged.
> >
> > Note that it requires firmware with SMCCC v1.2 support in the
> > underlying platform.
> >
> 
> I have a few questions to everyone:
> Is there any comprehensive documentation about how to use this? Things
> like where in Linux can I get the ARCH_SOC_ID and how are the ID's mapped
> to socs?
Kernel patch is at [1] - merged in Kernel 5.9 Makes use of SMCCC v1.2, SMCCC_ARCH_SOC_ID call.
SMCCC v1.2 spec is at [2]. 

The SoC ID becomes available at:
/sys/devices/socX/family
/sys/devices/socX/soc_id
/sys/devices/socX/revision

SOC_ID makes use of JEP-106 code [3] for the SiP which makes it unique.

[1] https://lkml.org/lkml/2020/6/25/208
[2] https://developer.arm.com/documentation/den0028/c
[3] https://developer.arm.com/documentation/ka001301/1-0

> If the ID mappings are not public then the plaform owners would need to
> add the support, right?
I agree here. There are firmware changes required and we do not know the IDs as well. It will be good for Marvell/NXP/NVIDIA to do this.
Pavan, is this ok for you?
  
Pavan Nikhilesh Bhagavatula Jan. 27, 2021, 3:02 p.m. UTC | #17
>-----Original Message-----
>From: dev <dev-bounces@dpdk.org> On Behalf Of Honnappa
>Nagarahalli
>Sent: Wednesday, January 27, 2021 7:11 PM
>To: Juraj Linkeš <juraj.linkes@pantheon.tech>; Pavan Nikhilesh
>Bhagavatula <pbhagavatula@marvell.com>;
>bruce.richardson@intel.com; Ruifeng Wang
><Ruifeng.Wang@arm.com>; Phil Yang <Phil.Yang@arm.com>;
>vcchunga@amazon.com; Dharmik Thakkar
><Dharmik.Thakkar@arm.com>; jerinjacobk@gmail.com;
>hemant.agrawal@nxp.com; Ajit Khaparde
>(ajit.khaparde@broadcom.com) <ajit.khaparde@broadcom.com>;
>ferruh.yigit@intel.com
>Cc: dev@dpdk.org; nd <nd@arm.com>; Honnappa Nagarahalli
><Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>
>Subject: [EXT] Re: [dpdk-dev] [PATCH v14 00/12] Arm build options
>rework
>
>External Email
>
>----------------------------------------------------------------------
><snip>
>
>> >
>> > > >
>> > > >v14:
>> > > >Resent for retesting (because of suspected false negative).
>> > > >
>> > > >Series Acked-by: Jerin Jacob <jerinj@marvell.com> Series Tested-
>by:
>> > > >Jerin Jacob <jerinj@marvell.com> Series Tested-by: Dharmik
>Thakkar
>> > > ><dharmik.thakkar@arm.com> Series Tested-by: Vimal Chungath
>> > > ><vcchunga@amazon.com>
>> > > >
>> > >
>> > > Series Tested-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>> > >
>> > > Also, I believe we need to take a look at ARCH_SOC_ID support
>coming
>> > > up in
>> > > 5.9 kernel.
>> > >
>> > > https://urldefense.proofpoint.com/v2/url?u=https-
>3A__lore.kernel.org_linux-2Darm-2Dkernel_20200506164411.3284-
>2D1-
>2D&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=1cjuAHrGh745jHNmj
>2fD85sUMIJ2IPIDsIJzo6FN6Z0&m=A7kOn5DYMe_WZiykt8BVATgv7-
>qTVhfo4iSgHBg-
>BTo&s=dslFI0W1qZdDxkbXDMTzPSUzlKMFfMUsE6tllPaF9Oc&e=
>> > > sudeep.holla@arm.com/
>> >
>> > Agree, it can come after this series is merged.
>> >
>> > Note that it requires firmware with SMCCC v1.2 support in the
>> > underlying platform.
>> >
>>
>> I have a few questions to everyone:
>> Is there any comprehensive documentation about how to use this?
>Things
>> like where in Linux can I get the ARCH_SOC_ID and how are the ID's
>mapped
>> to socs?
>Kernel patch is at [1] - merged in Kernel 5.9 Makes use of SMCCC v1.2,
>SMCCC_ARCH_SOC_ID call.
>SMCCC v1.2 spec is at [2].
>
>The SoC ID becomes available at:
>/sys/devices/socX/family
>/sys/devices/socX/soc_id
>/sys/devices/socX/revision
>
>SOC_ID makes use of JEP-106 code [3] for the SiP which makes it
>unique.
>
>[1] https://urldefense.proofpoint.com/v2/url?u=https-
>3A__lkml.org_lkml_2020_6_25_208&d=DwIGaQ&c=nKjWec2b6R0mOy
>Paz7xtfQ&r=1cjuAHrGh745jHNmj2fD85sUMIJ2IPIDsIJzo6FN6Z0&m=A7k
>On5DYMe_WZiykt8BVATgv7-qTVhfo4iSgHBg-
>BTo&s=5xXdzD7DpbcDeG81JVGZf396EFNbV0rSl00hRLuZJBc&e=
>[2] https://urldefense.proofpoint.com/v2/url?u=https-
>3A__developer.arm.com_documentation_den0028_c&d=DwIGaQ&c=n
>KjWec2b6R0mOyPaz7xtfQ&r=1cjuAHrGh745jHNmj2fD85sUMIJ2IPIDsIJz
>o6FN6Z0&m=A7kOn5DYMe_WZiykt8BVATgv7-qTVhfo4iSgHBg-
>BTo&s=DFG7a9oFDFczCcODfR0wVGjkLQXwMU19q_ogaFVw90I&e=
>[3] https://urldefense.proofpoint.com/v2/url?u=https-
>3A__developer.arm.com_documentation_ka001301_1-
>2D0&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=1cjuAHrGh745jHNm
>j2fD85sUMIJ2IPIDsIJzo6FN6Z0&m=A7kOn5DYMe_WZiykt8BVATgv7-
>qTVhfo4iSgHBg-
>BTo&s=GQsJJuft9k1iooO8VNNIM8ZUUgGjCEPB6eRxN3qm62U&e=
>
>> If the ID mappings are not public then the plaform owners would need
>to
>> add the support, right?
>I agree here. There are firmware changes required and we do not know
>the IDs as well. It will be good for Marvell/NXP/NVIDIA to do this.
>Pavan, is this ok for you?

Yeah, I believe we need fallback to the current mechanisms if SOC_ID support 
is not present or platform owners have not updated them.
  
Hemant Agrawal Jan. 27, 2021, 3:12 p.m. UTC | #18
> 
> >-----Original Message-----
> >From: dev <dev-bounces@dpdk.org> On Behalf Of Honnappa Nagarahalli
> >Sent: Wednesday, January 27, 2021 7:11 PM
> >To: Juraj Linkeš <juraj.linkes@pantheon.tech>; Pavan Nikhilesh
> >Bhagavatula <pbhagavatula@marvell.com>; bruce.richardson@intel.com;
> >Ruifeng Wang <Ruifeng.Wang@arm.com>; Phil Yang <Phil.Yang@arm.com>;
> >vcchunga@amazon.com; Dharmik Thakkar <Dharmik.Thakkar@arm.com>;
> >jerinjacobk@gmail.com; hemant.agrawal@nxp.com; Ajit Khaparde
> >(ajit.khaparde@broadcom.com) <ajit.khaparde@broadcom.com>;
> >ferruh.yigit@intel.com
> >Cc: dev@dpdk.org; nd <nd@arm.com>; Honnappa Nagarahalli
> ><Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>
> >Subject: [EXT] Re: [dpdk-dev] [PATCH v14 00/12] Arm build options
> >rework
> >
> >External Email
> >
> >----------------------------------------------------------------------
> ><snip>
> >
> >> >
> >> > > >
> >> > > >v14:
> >> > > >Resent for retesting (because of suspected false negative).
> >> > > >
> >> > > >Series Acked-by: Jerin Jacob <jerinj@marvell.com> Series Tested-
> >by:
> >> > > >Jerin Jacob <jerinj@marvell.com> Series Tested-by: Dharmik
> >Thakkar
> >> > > ><dharmik.thakkar@arm.com> Series Tested-by: Vimal Chungath
> >> > > ><vcchunga@amazon.com>
> >> > > >
> >> > >
> >> > > Series Tested-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >> > >
> >> > > Also, I believe we need to take a look at ARCH_SOC_ID support
> >coming
> >> > > up in
> >> > > 5.9 kernel.
> >> > >
> >> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2
> >> > > Furldefense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps-
> &amp;data=04%7C
> >> > >
> 01%7Chemant.agrawal%40nxp.com%7Cf05a2623ec9347bd254708d8c2d4896
> c%
> >> > >
> 7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637473565383043785
> %7
> >> > >
> CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB
> Ti
> >> > >
> I6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=o7%2BXNileFgI2By08IHv
> EY
> >> > > PWrDjRikXlfFtal1MCdyCM%3D&amp;reserved=0
> >3A__lore.kernel.org_linux-2Darm-2Dkernel_20200506164411.3284-
> >2D1-
> >2D&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=1cjuAHrGh745jHNmj
> >2fD85sUMIJ2IPIDsIJzo6FN6Z0&m=A7kOn5DYMe_WZiykt8BVATgv7-
> >qTVhfo4iSgHBg-
> >BTo&s=dslFI0W1qZdDxkbXDMTzPSUzlKMFfMUsE6tllPaF9Oc&e=
> >> > > sudeep.holla@arm.com/
> >> >
> >> > Agree, it can come after this series is merged.
> >> >
> >> > Note that it requires firmware with SMCCC v1.2 support in the
> >> > underlying platform.
> >> >
> >>
> >> I have a few questions to everyone:
> >> Is there any comprehensive documentation about how to use this?
> >Things
> >> like where in Linux can I get the ARCH_SOC_ID and how are the ID's
> >mapped
> >> to socs?
> >Kernel patch is at [1] - merged in Kernel 5.9 Makes use of SMCCC v1.2,
> >SMCCC_ARCH_SOC_ID call.
> >SMCCC v1.2 spec is at [2].
> >
> >The SoC ID becomes available at:
> >/sys/devices/socX/family
> >/sys/devices/socX/soc_id
> >/sys/devices/socX/revision
> >
> >SOC_ID makes use of JEP-106 code [3] for the SiP which makes it unique.
> >
> >[1]
> >https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Furld
> e
> >fense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps-
> &amp;data=04%7C01%7Chemant.
> >agrawal%40nxp.com%7Cf05a2623ec9347bd254708d8c2d4896c%7C686ea1d
> 3bc2b4c6f
> >a92cd99c5c301635%7C0%7C0%7C637473565383043785%7CUnknown%7CT
> WFpbGZsb3d8e
> >yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> D%7C100
> >0&amp;sdata=o7%2BXNileFgI2By08IHvEYPWrDjRikXlfFtal1MCdyCM%3D&a
> mp;reserv
> >ed=0 3A__lkml.org_lkml_2020_6_25_208&d=DwIGaQ&c=nKjWec2b6R0mOy
> >Paz7xtfQ&r=1cjuAHrGh745jHNmj2fD85sUMIJ2IPIDsIJzo6FN6Z0&m=A7k
> >On5DYMe_WZiykt8BVATgv7-qTVhfo4iSgHBg-
> >BTo&s=5xXdzD7DpbcDeG81JVGZf396EFNbV0rSl00hRLuZJBc&e=
> >[2]
> >https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Furld
> e
> >fense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps-
> &amp;data=04%7C01%7Chemant.
> >agrawal%40nxp.com%7Cf05a2623ec9347bd254708d8c2d4896c%7C686ea1d
> 3bc2b4c6f
> >a92cd99c5c301635%7C0%7C0%7C637473565383043785%7CUnknown%7CT
> WFpbGZsb3d8e
> >yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> D%7C100
> >0&amp;sdata=o7%2BXNileFgI2By08IHvEYPWrDjRikXlfFtal1MCdyCM%3D&a
> mp;reserv
> >ed=0
> 3A__developer.arm.com_documentation_den0028_c&d=DwIGaQ&c=n
> >KjWec2b6R0mOyPaz7xtfQ&r=1cjuAHrGh745jHNmj2fD85sUMIJ2IPIDsIJz
> >o6FN6Z0&m=A7kOn5DYMe_WZiykt8BVATgv7-qTVhfo4iSgHBg-
> >BTo&s=DFG7a9oFDFczCcODfR0wVGjkLQXwMU19q_ogaFVw90I&e=
> >[3]
> >https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Furld
> e
> >fense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps-
> &amp;data=04%7C01%7Chemant.
> >agrawal%40nxp.com%7Cf05a2623ec9347bd254708d8c2d4896c%7C686ea1d
> 3bc2b4c6f
> >a92cd99c5c301635%7C0%7C0%7C637473565383043785%7CUnknown%7CT
> WFpbGZsb3d8e
> >yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> D%7C100
> >0&amp;sdata=o7%2BXNileFgI2By08IHvEYPWrDjRikXlfFtal1MCdyCM%3D&a
> mp;reserv
> >ed=0
> >3A__developer.arm.com_documentation_ka001301_1-
> >2D0&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=1cjuAHrGh745jHNm
> >j2fD85sUMIJ2IPIDsIJzo6FN6Z0&m=A7kOn5DYMe_WZiykt8BVATgv7-
> >qTVhfo4iSgHBg-
> >BTo&s=GQsJJuft9k1iooO8VNNIM8ZUUgGjCEPB6eRxN3qm62U&e=
> >
> >> If the ID mappings are not public then the plaform owners would need
> >to
> >> add the support, right?
> >I agree here. There are firmware changes required and we do not know
> >the IDs as well. It will be good for Marvell/NXP/NVIDIA to do this.
> >Pavan, is this ok for you?
> 
> Yeah, I believe we need fallback to the current mechanisms if SOC_ID
> support is not present or platform owners have not updated them.

[Hemant]  +1
  
Honnappa Nagarahalli Jan. 27, 2021, 3:18 p.m. UTC | #19
<snip>

> > >
> > >> >
> > >> > > >
> > >> > > >v14:
> > >> > > >Resent for retesting (because of suspected false negative).
> > >> > > >
> > >> > > >Series Acked-by: Jerin Jacob <jerinj@marvell.com> Series
> > >> > > >Tested-
> > >by:
> > >> > > >Jerin Jacob <jerinj@marvell.com> Series Tested-by: Dharmik
> > >Thakkar
> > >> > > ><dharmik.thakkar@arm.com> Series Tested-by: Vimal Chungath
> > >> > > ><vcchunga@amazon.com>
> > >> > > >
> > >> > >
> > >> > > Series Tested-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > >> > >
> > >> > > Also, I believe we need to take a look at ARCH_SOC_ID support
> > >coming
> > >> > > up in
> > >> > > 5.9 kernel.
> > >> > >
> > >> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F
> > >> > > %2
> > >> > > Furldefense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps-
> > &amp;data=04%7C
> > >> > >
> >
> 01%7Chemant.agrawal%40nxp.com%7Cf05a2623ec9347bd254708d8c2d4896
> > c%
> > >> > >
> > 7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637473565383043785
> > %7
> > >> > >
> >
> CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJ
> B
> > Ti
> > >> > >
> >
> I6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=o7%2BXNileFgI2By08IHv
> > EY
> > >> > > PWrDjRikXlfFtal1MCdyCM%3D&amp;reserved=0
> > >3A__lore.kernel.org_linux-2Darm-2Dkernel_20200506164411.3284-
> > >2D1-
> > >2D&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=1cjuAHrGh745jHNmj
> > >2fD85sUMIJ2IPIDsIJzo6FN6Z0&m=A7kOn5DYMe_WZiykt8BVATgv7-
> > >qTVhfo4iSgHBg-
> > >BTo&s=dslFI0W1qZdDxkbXDMTzPSUzlKMFfMUsE6tllPaF9Oc&e=
> > >> > > sudeep.holla@arm.com/
> > >> >
> > >> > Agree, it can come after this series is merged.
> > >> >
> > >> > Note that it requires firmware with SMCCC v1.2 support in the
> > >> > underlying platform.
> > >> >
> > >>
> > >> I have a few questions to everyone:
> > >> Is there any comprehensive documentation about how to use this?
> > >Things
> > >> like where in Linux can I get the ARCH_SOC_ID and how are the ID's
> > >mapped
> > >> to socs?
> > >Kernel patch is at [1] - merged in Kernel 5.9 Makes use of SMCCC
> > >v1.2, SMCCC_ARCH_SOC_ID call.
> > >SMCCC v1.2 spec is at [2].
> > >
> > >The SoC ID becomes available at:
> > >/sys/devices/socX/family
> > >/sys/devices/socX/soc_id
> > >/sys/devices/socX/revision
> > >
> > >SOC_ID makes use of JEP-106 code [3] for the SiP which makes it unique.
> > >
> > >[1]
> > >https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Furl
> > >d
> > e
> > >fense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps-
> > &amp;data=04%7C01%7Chemant.
> >
> >agrawal%40nxp.com%7Cf05a2623ec9347bd254708d8c2d4896c%7C686ea1d
> > 3bc2b4c6f
> > >a92cd99c5c301635%7C0%7C0%7C637473565383043785%7CUnknown%7CT
> > WFpbGZsb3d8e
> >
> >yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> > D%7C100
> >
> >0&amp;sdata=o7%2BXNileFgI2By08IHvEYPWrDjRikXlfFtal1MCdyCM%3D&a
> > mp;reserv
> > >ed=0
> 3A__lkml.org_lkml_2020_6_25_208&d=DwIGaQ&c=nKjWec2b6R0mOy
> > >Paz7xtfQ&r=1cjuAHrGh745jHNmj2fD85sUMIJ2IPIDsIJzo6FN6Z0&m=A7k
> > >On5DYMe_WZiykt8BVATgv7-qTVhfo4iSgHBg-
> > >BTo&s=5xXdzD7DpbcDeG81JVGZf396EFNbV0rSl00hRLuZJBc&e=
> > >[2]
> > >https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Furl
> > >d
> > e
> > >fense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps-
> > &amp;data=04%7C01%7Chemant.
> >
> >agrawal%40nxp.com%7Cf05a2623ec9347bd254708d8c2d4896c%7C686ea1d
> > 3bc2b4c6f
> > >a92cd99c5c301635%7C0%7C0%7C637473565383043785%7CUnknown%7CT
> > WFpbGZsb3d8e
> >
> >yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> > D%7C100
> >
> >0&amp;sdata=o7%2BXNileFgI2By08IHvEYPWrDjRikXlfFtal1MCdyCM%3D&a
> > mp;reserv
> > >ed=0
> > 3A__developer.arm.com_documentation_den0028_c&d=DwIGaQ&c=n
> > >KjWec2b6R0mOyPaz7xtfQ&r=1cjuAHrGh745jHNmj2fD85sUMIJ2IPIDsIJz
> > >o6FN6Z0&m=A7kOn5DYMe_WZiykt8BVATgv7-qTVhfo4iSgHBg-
> > >BTo&s=DFG7a9oFDFczCcODfR0wVGjkLQXwMU19q_ogaFVw90I&e=
> > >[3]
> > >https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Furl
> > >d
> > e
> > >fense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps-
> > &amp;data=04%7C01%7Chemant.
> >
> >agrawal%40nxp.com%7Cf05a2623ec9347bd254708d8c2d4896c%7C686ea1d
> > 3bc2b4c6f
> > >a92cd99c5c301635%7C0%7C0%7C637473565383043785%7CUnknown%7CT
> > WFpbGZsb3d8e
> >
> >yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> > D%7C100
> >
> >0&amp;sdata=o7%2BXNileFgI2By08IHvEYPWrDjRikXlfFtal1MCdyCM%3D&a
> > mp;reserv
> > >ed=0
> > >3A__developer.arm.com_documentation_ka001301_1-
> > >2D0&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=1cjuAHrGh745jHNm
> > >j2fD85sUMIJ2IPIDsIJzo6FN6Z0&m=A7kOn5DYMe_WZiykt8BVATgv7-
> > >qTVhfo4iSgHBg-
> > >BTo&s=GQsJJuft9k1iooO8VNNIM8ZUUgGjCEPB6eRxN3qm62U&e=
> > >
> > >> If the ID mappings are not public then the plaform owners would
> > >> need
> > >to
> > >> add the support, right?
> > >I agree here. There are firmware changes required and we do not know
> > >the IDs as well. It will be good for Marvell/NXP/NVIDIA to do this.
> > >Pavan, is this ok for you?
> >
> > Yeah, I believe we need fallback to the current mechanisms if SOC_ID
> > support is not present or platform owners have not updated them.
> 
> [Hemant]  +1
> 
Agree.
Just to be clear, my ask is, can any of you take this work as Arm will not have all the parts to test this?
  
Juraj Linkeš Jan. 29, 2021, 8:41 a.m. UTC | #20
> > 3) We need to build kni against the aarch64 kernel headers, but it
> > fails. It appears that kernel/linux/kni/meson.build doesn’t pass any
> > cross-compile flags in the make command it creates. The diff below
> > shows how I hardcoded it to get it to work for now. Thoughts on how to
> > do this right? meson has the path (in
> > $PATH) and the binary prefixs (in ‘[binaries]’ in the cross file). It
> > must know the arch, too.
> >
> > --- a/kernel/linux/kni/meson.build
> > +++ b/kernel/linux/kni/meson.build
> > @@ -14,6 +14,8 @@ custom_target('rte_kni',
> >         input: kni_sources,
> >         output: 'rte_kni.ko',
> >         command: ['make', '-j4', '-C', kernel_dir + '/build',
> > +
> > + 'CROSS_COMPILE=/tool/toolchain/aarch64-1.1/bin/aarch64-linux-
> > gnu-',
> > +               'ARCH=aarch64',
> >                 'M=' + meson.current_build_dir(),
> >                 'src=' + meson.current_source_dir(),
> >                 'MODULE_CFLAGS=-include ' + meson.source_root() +
> > '/config/rte_config.h' +
> >
> 
> Yes, Meson should know these. I'll submit this separately (in a new series), as
> this series is getting pretty big and this fix is actually unrelated.
> 

I tried to figure out how to implement this and I've ran into a brick wall - there's no way to figure out the full name of the cross-compiler. I found only one way to get the cross compiler:
meson.get_compiler('c', native:false).get_id()

But that only retuns 'gcc', not the full binary name (c = 'aarch64-linux-gnu-gcc' in cross file).

Bruce, any ideas on how to get the full name? If I understand the sources right, we aren't able to access arbitrary object attributes, just those, which are specifically exposed.

Maybe we could do a workaround? Either if cross compiling and the compiler is gcc, use the 'aarch64-linux-gnu-' prefix or just put the prefix into the cross file as an additional property?

> > I will try to get to a full review soon.
> >
> > Thank you,
> > Andrew
  
Bruce Richardson Jan. 29, 2021, 9:45 a.m. UTC | #21
On Fri, Jan 29, 2021 at 08:41:45AM +0000, Juraj Linkeš wrote:
> > > 3) We need to build kni against the aarch64 kernel headers, but it
> > > fails. It appears that kernel/linux/kni/meson.build doesn’t pass any
> > > cross-compile flags in the make command it creates. The diff below
> > > shows how I hardcoded it to get it to work for now. Thoughts on how
> > > to do this right? meson has the path (in $PATH) and the binary
> > > prefixs (in ‘[binaries]’ in the cross file). It must know the arch,
> > > too.
> > >
> > > --- a/kernel/linux/kni/meson.build +++ b/kernel/linux/kni/meson.build
> > > @@ -14,6 +14,8 @@ custom_target('rte_kni', input: kni_sources,
> > > output: 'rte_kni.ko', command: ['make', '-j4', '-C', kernel_dir +
> > > '/build', + +
> > > 'CROSS_COMPILE=/tool/toolchain/aarch64-1.1/bin/aarch64-linux- gnu-',
> > > +               'ARCH=aarch64', 'M=' + meson.current_build_dir(),
> > > 'src=' + meson.current_source_dir(), 'MODULE_CFLAGS=-include ' +
> > > meson.source_root() + '/config/rte_config.h' +
> > >
> > 
> > Yes, Meson should know these. I'll submit this separately (in a new
> > series), as this series is getting pretty big and this fix is actually
> > unrelated.
> > 
> 
> I tried to figure out how to implement this and I've ran into a brick
> wall - there's no way to figure out the full name of the cross-compiler.
> I found only one way to get the cross compiler: meson.get_compiler('c',
> native:false).get_id()
> 
> But that only retuns 'gcc', not the full binary name (c =
> 'aarch64-linux-gnu-gcc' in cross file).
> 
> Bruce, any ideas on how to get the full name? If I understand the sources
> right, we aren't able to access arbitrary object attributes, just those,
> which are specifically exposed.
> 
> Maybe we could do a workaround? Either if cross compiling and the
> compiler is gcc, use the 'aarch64-linux-gnu-' prefix or just put the
> prefix into the cross file as an additional property?
> 
According to the cross-file/native-file documentation[1], the files in the
binaries section can be queried using the "find_program()" call. That then
returns an external program object rather than a compiler one, so the full
path can be got. Unfortunately, find_program('c') doesn't seem to work for
the non-cross-compiled case, so it will need to be conditional on
cross-compilation.

Example I tested:

<code change added>
+myc = find_program('c')
+message('find_program(\'c\') = ' + myc.full_path())
+

<output when run>
~/dpdk.org/__BUILDS/build-arm64-host-clang$ ninja
[0/1] Regenerating build files.
....
Target machine cpu family: aarch64
Target machine cpu: armv8-a
Program aarch64-linux-gnu-gcc found: YES
WARNING: Project targeting '>= 0.47.1' but tried to use feature introduced in '0.55.0': ExternalProgram.full_path.
Message: find_program('c') = /home/bruce/Downloads/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc
...

/Bruce


[1] https://mesonbuild.com/Machine-files.html#binaries
  
Juraj Linkeš Jan. 29, 2021, 10:07 a.m. UTC | #22
> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, January 29, 2021 10:46 AM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: Andrew Boyer <aboyer@pensando.io>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; thomas@monjalon.net; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; Phil Yang <Phil.Yang@arm.com>;
> vcchunga@amazon.com; Dharmik Thakkar <Dharmik.Thakkar@arm.com>;
> jerinjacobk@gmail.com; hemant.agrawal@nxp.com; Ajit Khaparde
> (ajit.khaparde@broadcom.com) <ajit.khaparde@broadcom.com>;
> ferruh.yigit@intel.com; dev@dpdk.org; nd <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v14 00/12] Arm build options rework
> 
> On Fri, Jan 29, 2021 at 08:41:45AM +0000, Juraj Linkeš wrote:
> > > > 3) We need to build kni against the aarch64 kernel headers, but it
> > > > fails. It appears that kernel/linux/kni/meson.build doesn’t pass
> > > > any cross-compile flags in the make command it creates. The diff
> > > > below shows how I hardcoded it to get it to work for now. Thoughts
> > > > on how to do this right? meson has the path (in $PATH) and the
> > > > binary prefixs (in ‘[binaries]’ in the cross file). It must know
> > > > the arch, too.
> > > >
> > > > --- a/kernel/linux/kni/meson.build +++
> > > > b/kernel/linux/kni/meson.build @@ -14,6 +14,8 @@
> > > > custom_target('rte_kni', input: kni_sources,
> > > > output: 'rte_kni.ko', command: ['make', '-j4', '-C', kernel_dir +
> > > > '/build', + +
> > > > 'CROSS_COMPILE=/tool/toolchain/aarch64-1.1/bin/aarch64-linux-
> > > > gnu-',
> > > > +               'ARCH=aarch64', 'M=' + meson.current_build_dir(),
> > > > 'src=' + meson.current_source_dir(), 'MODULE_CFLAGS=-include ' +
> > > > meson.source_root() + '/config/rte_config.h' +
> > > >
> > >
> > > Yes, Meson should know these. I'll submit this separately (in a new
> > > series), as this series is getting pretty big and this fix is
> > > actually unrelated.
> > >
> >
> > I tried to figure out how to implement this and I've ran into a brick
> > wall - there's no way to figure out the full name of the cross-compiler.
> > I found only one way to get the cross compiler:
> > meson.get_compiler('c',
> > native:false).get_id()
> >
> > But that only retuns 'gcc', not the full binary name (c =
> > 'aarch64-linux-gnu-gcc' in cross file).
> >
> > Bruce, any ideas on how to get the full name? If I understand the
> > sources right, we aren't able to access arbitrary object attributes,
> > just those, which are specifically exposed.
> >
> > Maybe we could do a workaround? Either if cross compiling and the
> > compiler is gcc, use the 'aarch64-linux-gnu-' prefix or just put the
> > prefix into the cross file as an additional property?
> >
> According to the cross-file/native-file documentation[1], the files in the binaries
> section can be queried using the "find_program()" call. That then returns an
> external program object rather than a compiler one, so the full path can be got.
> Unfortunately, find_program('c') doesn't seem to work for the non-cross-
> compiled case, so it will need to be conditional on cross-compilation.
> 

Thanks, this is great, I'll use this.
We don't need it for native builds. But it's strange that it doesn't work for those - the docs say it does.

> Example I tested:
> 
> <code change added>
> +myc = find_program('c')
> +message('find_program(\'c\') = ' + myc.full_path())
> +
> 
> <output when run>
> ~/dpdk.org/__BUILDS/build-arm64-host-clang$ ninja [0/1] Regenerating build
> files.
> ....
> Target machine cpu family: aarch64
> Target machine cpu: armv8-a
> Program aarch64-linux-gnu-gcc found: YES
> WARNING: Project targeting '>= 0.47.1' but tried to use feature introduced in
> '0.55.0': ExternalProgram.full_path.
> Message: find_program('c') = /home/bruce/Downloads/gcc-arm-8.3-2019.03-
> x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc
> ...
> 
> /Bruce
> 
> 
> [1] https://mesonbuild.com/Machine-files.html#binaries
  
Bruce Richardson Jan. 29, 2021, 10:09 a.m. UTC | #23
On Fri, Jan 29, 2021 at 10:07:30AM +0000, Juraj Linkeš wrote:
> 
> 
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Friday, January 29, 2021 10:46 AM
> > To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > Cc: Andrew Boyer <aboyer@pensando.io>; Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com>; thomas@monjalon.net; Ruifeng Wang
> > <Ruifeng.Wang@arm.com>; Phil Yang <Phil.Yang@arm.com>;
> > vcchunga@amazon.com; Dharmik Thakkar <Dharmik.Thakkar@arm.com>;
> > jerinjacobk@gmail.com; hemant.agrawal@nxp.com; Ajit Khaparde
> > (ajit.khaparde@broadcom.com) <ajit.khaparde@broadcom.com>;
> > ferruh.yigit@intel.com; dev@dpdk.org; nd <nd@arm.com>
> > Subject: Re: [dpdk-dev] [PATCH v14 00/12] Arm build options rework
> > 
> > On Fri, Jan 29, 2021 at 08:41:45AM +0000, Juraj Linkeš wrote:
> > > > > 3) We need to build kni against the aarch64 kernel headers, but it
> > > > > fails. It appears that kernel/linux/kni/meson.build doesn’t pass
> > > > > any cross-compile flags in the make command it creates. The diff
> > > > > below shows how I hardcoded it to get it to work for now. Thoughts
> > > > > on how to do this right? meson has the path (in $PATH) and the
> > > > > binary prefixs (in ‘[binaries]’ in the cross file). It must know
> > > > > the arch, too.
> > > > >
> > > > > --- a/kernel/linux/kni/meson.build +++
> > > > > b/kernel/linux/kni/meson.build @@ -14,6 +14,8 @@
> > > > > custom_target('rte_kni', input: kni_sources,
> > > > > output: 'rte_kni.ko', command: ['make', '-j4', '-C', kernel_dir +
> > > > > '/build', + +
> > > > > 'CROSS_COMPILE=/tool/toolchain/aarch64-1.1/bin/aarch64-linux-
> > > > > gnu-',
> > > > > +               'ARCH=aarch64', 'M=' + meson.current_build_dir(),
> > > > > 'src=' + meson.current_source_dir(), 'MODULE_CFLAGS=-include ' +
> > > > > meson.source_root() + '/config/rte_config.h' +
> > > > >
> > > >
> > > > Yes, Meson should know these. I'll submit this separately (in a new
> > > > series), as this series is getting pretty big and this fix is
> > > > actually unrelated.
> > > >
> > >
> > > I tried to figure out how to implement this and I've ran into a brick
> > > wall - there's no way to figure out the full name of the cross-compiler.
> > > I found only one way to get the cross compiler:
> > > meson.get_compiler('c',
> > > native:false).get_id()
> > >
> > > But that only retuns 'gcc', not the full binary name (c =
> > > 'aarch64-linux-gnu-gcc' in cross file).
> > >
> > > Bruce, any ideas on how to get the full name? If I understand the
> > > sources right, we aren't able to access arbitrary object attributes,
> > > just those, which are specifically exposed.
> > >
> > > Maybe we could do a workaround? Either if cross compiling and the
> > > compiler is gcc, use the 'aarch64-linux-gnu-' prefix or just put the
> > > prefix into the cross file as an additional property?
> > >
> > According to the cross-file/native-file documentation[1], the files in the binaries
> > section can be queried using the "find_program()" call. That then returns an
> > external program object rather than a compiler one, so the full path can be got.
> > Unfortunately, find_program('c') doesn't seem to work for the non-cross-
> > compiled case, so it will need to be conditional on cross-compilation.
> > 
> 
> Thanks, this is great, I'll use this.
> We don't need it for native builds. But it's strange that it doesn't work for those - the docs say it does.
> 
If probably works if a native-file rather than a cross-file is passed.
Without a native file, meson is probably looking for a program called "c"
on the OS, rather than recognising that it's the C compiler being looked
for.

/Bruce