[dpdk-dev] [PATCH 0/6] build: fix build for arm64

[dpdk-dev] [PATCH 0/6] build: fix build for arm64

[Yongseok Koh]

This patchset depends on
"meson: add infra to support machine specific flags" [1]

[1] http://patches.dpdk.org/patch/52606/

Yongseok Koh (6):
  meson: disable octeontx for buggy compilers on arm64
  meson: change default cache line size for cortex-a72
  net/mlx: fix library search in meson build
  meson: add Mellanox BlueField cross-compile config
  build: add option for armv8 crypto extension
  mk: disable armv8 crypto extension for Mellanox BlueField

 config/arm/arm64_bluefield_linux_gcc          | 16 ++++++++++++++++
 config/arm/meson.build                        | 18 +++++++++++-------
 config/common_armv8a_linux                    |  1 +
 config/defconfig_arm64-bluefield-linuxapp-gcc |  6 ++++++
 drivers/crypto/armv8/Makefile                 |  4 ++++
 drivers/event/meson.build                     |  6 +++++-
 drivers/net/mlx4/meson.build                  | 19 +++++++++++--------
 drivers/net/mlx5/meson.build                  | 19 +++++++++++--------
 meson_options.txt                             |  2 ++
 mk/machine/armv8a/rte.vars.mk                 |  4 ++++
 10 files changed, 71 insertions(+), 24 deletions(-)
 create mode 100644 config/arm/arm64_bluefield_linux_gcc

-- 
2.21.0.196.g041f5ea

[dpdk-dev] [PATCH 1/6] meson: disable octeontx for buggy compilers on arm64

[Yongseok Koh]

Disable octeontx for gcc 4.8.5 as compiler is emitting "internal compiler
error" for aarch64

Fixes: bd77f2d64c44 ("event/octeontx: build with meson")
Fixes: 4f760550a093 ("mk: disable OcteonTx for buggy compilers")
Fixes: f3af3e44a444 ("mk: disable OcteonTx for buggy compilers only on arm64")
Cc: pbhagavatula@marvell.com
Cc: jerinj@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/event/meson.build | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/event/meson.build b/drivers/event/meson.build
index 836ecbb74b..d364871d15 100644
--- a/drivers/event/meson.build
+++ b/drivers/event/meson.build
@@ -1,7 +1,11 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation
 
-drivers = ['dpaa', 'dpaa2', 'octeontx', 'opdl', 'skeleton', 'sw', 'dsw']
+drivers = ['dpaa', 'dpaa2', 'opdl', 'skeleton', 'sw', 'dsw']
+if (toolchain == 'gcc' and cc.version().version_compare('>=4.8.6') and
+    dpdk_conf.has('RTE_ARCH_ARM64'))
+	drivers += 'octeontx'
+endif
 std_deps = ['eventdev', 'kvargs']
 config_flag_fmt = 'RTE_LIBRTE_@0@_EVENTDEV_PMD'
 driver_name_fmt = 'rte_pmd_@0@_event'
-- 
2.21.0.196.g041f5ea

Re: [dpdk-dev] [EXT] [PATCH 1/6] meson: disable octeontx for buggy compilers on arm64

[Pavan Nikhilesh Bhagavatula]

Hi Yongseok,

>----------------------------------------------------------------------
>Disable octeontx for gcc 4.8.5 as compiler is emitting "internal compiler error"
>for aarch64
>
>Fixes: bd77f2d64c44 ("event/octeontx: build with meson")
>Fixes: 4f760550a093 ("mk: disable OcteonTx for buggy compilers")
>Fixes: f3af3e44a444 ("mk: disable OcteonTx for buggy compilers only on
>arm64")
>Cc: pbhagavatula@marvell.com
>Cc: jerinj@marvell.com
>Cc: stable@dpdk.org
>
>Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>---
> drivers/event/meson.build | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/event/meson.build b/drivers/event/meson.build index
>836ecbb74b..d364871d15 100644
>--- a/drivers/event/meson.build
>+++ b/drivers/event/meson.build
>@@ -1,7 +1,11 @@
> # SPDX-License-Identifier: BSD-3-Clause  # Copyright(c) 2017 Intel Corporation
>
>-drivers = ['dpaa', 'dpaa2', 'octeontx', 'opdl', 'skeleton', 'sw', 'dsw']
>+drivers = ['dpaa', 'dpaa2', 'opdl', 'skeleton', 'sw', 'dsw'] if
>+(toolchain == 'gcc' and cc.version().version_compare('>=4.8.6') and
>+    dpdk_conf.has('RTE_ARCH_ARM64'))

Can we make this similar to MAKEFILE[1] case where octeontx is enabled for everycase except when 
We are compiling for ARCH_ARM64 is set and compiler is less than < 4.8.6?.
The reason being we want x86 CI to run (compilation part) to find any errors.

[1]
'
ifeq ($(CONFIG_RTE_ARCH), arm64)
        ifeq ($(shell test $(GCC_VERSION)$(GCC_PATCHLEVEL) -lt 486 && echo 1), 1)
                CONFIG_RTE_LIBRTE_PMD_OCTEONTX_SSOVF=d
                CONFIG_RTE_LIBRTE_OCTEONTX_MEMPOOL=d
                CONFIG_RTE_LIBRTE_OCTEONTX_PMD=d
        endif
        endif
'

>+	drivers += 'octeontx'
>+endif
> std_deps = ['eventdev', 'kvargs']
> config_flag_fmt = 'RTE_LIBRTE_@0@_EVENTDEV_PMD'
> driver_name_fmt = 'rte_pmd_@0@_event'
>--
>2.21.0.196.g041f5ea

Regards,
Pavan.

Re: [dpdk-dev] [EXT] [PATCH 1/6] meson: disable octeontx for buggy compilers on arm64

[Yongseok Koh]

> On Apr 12, 2019, at 10:52 PM, Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com> wrote:
> 
> Hi Yongseok,
> 
>> ----------------------------------------------------------------------
>> Disable octeontx for gcc 4.8.5 as compiler is emitting "internal compiler error"
>> for aarch64
>> 
>> Fixes: bd77f2d64c44 ("event/octeontx: build with meson")
>> Fixes: 4f760550a093 ("mk: disable OcteonTx for buggy compilers")
>> Fixes: f3af3e44a444 ("mk: disable OcteonTx for buggy compilers only on
>> arm64")
>> Cc: pbhagavatula@marvell.com
>> Cc: jerinj@marvell.com
>> Cc: stable@dpdk.org
>> 
>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>> ---
>> drivers/event/meson.build | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/event/meson.build b/drivers/event/meson.build index
>> 836ecbb74b..d364871d15 100644
>> --- a/drivers/event/meson.build
>> +++ b/drivers/event/meson.build
>> @@ -1,7 +1,11 @@
>> # SPDX-License-Identifier: BSD-3-Clause  # Copyright(c) 2017 Intel Corporation
>> 
>> -drivers = ['dpaa', 'dpaa2', 'octeontx', 'opdl', 'skeleton', 'sw', 'dsw']
>> +drivers = ['dpaa', 'dpaa2', 'opdl', 'skeleton', 'sw', 'dsw'] if
>> +(toolchain == 'gcc' and cc.version().version_compare('>=4.8.6') and
>> +    dpdk_conf.has('RTE_ARCH_ARM64'))
> 
> Can we make this similar to MAKEFILE[1] case where octeontx is enabled for everycase except when 
> We are compiling for ARCH_ARM64 is set and compiler is less than < 4.8.6?.
> The reason being we want x86 CI to run (compilation part) to find any errors.
> 
> [1]
> '
> ifeq ($(CONFIG_RTE_ARCH), arm64)
>        ifeq ($(shell test $(GCC_VERSION)$(GCC_PATCHLEVEL) -lt 486 && echo 1), 1)
>                CONFIG_RTE_LIBRTE_PMD_OCTEONTX_SSOVF=d
>                CONFIG_RTE_LIBRTE_OCTEONTX_MEMPOOL=d
>                CONFIG_RTE_LIBRTE_OCTEONTX_PMD=d
>        endif
>        endif
> '

OMG, I was very wrong about the patch. That was Friday and I was moving fast. :-)
Will fix it in v2.

thanks,
Yongseok

> 
>> +	drivers += 'octeontx'
>> +endif
>> std_deps = ['eventdev', 'kvargs']
>> config_flag_fmt = 'RTE_LIBRTE_@0@_EVENTDEV_PMD'
>> driver_name_fmt = 'rte_pmd_@0@_event'
>> --
>> 2.21.0.196.g041f5ea

[dpdk-dev] [PATCH 2/6] meson: change default cache line size for cortex-a72

[Yongseok Koh]

Per the email discussion [1], the default cache line size of armv8
cortex-a72 is changed to 64 bytes.

[1] https://mails.dpdk.org/archives/dev/2019-January/123218.html

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 config/arm/meson.build | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/config/arm/meson.build b/config/arm/meson.build
index e00b894523..73c581948c 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -51,6 +51,8 @@ flags_dpaa2 = [
 	['RTE_MAX_LCORE', 16],
 	['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]]
 flags_default_extra = []
+flags_cortex_a72_extra = [
+	['RTE_CACHE_LINE_SIZE', 64]]
 flags_thunderx_extra = [
 	['RTE_MACHINE', '"thunderx"'],
 	['RTE_USE_C11_MEM_MODEL', false]]
@@ -73,7 +75,7 @@ machine_args_generic = [
 	['0xd03', ['-mcpu=cortex-a53']],
 	['0xd04', ['-mcpu=cortex-a35']],
 	['0xd07', ['-mcpu=cortex-a57']],
-	['0xd08', ['-mcpu=cortex-a72']],
+	['0xd08', ['-mcpu=cortex-a72'], flags_cortex_a72_extra],
 	['0xd09', ['-mcpu=cortex-a73']],
 	['0xd0a', ['-mcpu=cortex-a75']]]
 
-- 
2.21.0.196.g041f5ea

Re: [dpdk-dev] [EXT] [PATCH 2/6] meson: change default cache line size for cortex-a72

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Yongseok Koh <yskoh@mellanox.com>
> Sent: Saturday, April 13, 2019 4:55 AM
> To: bruce.richardson@intel.com; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>; shahafs@mellanox.com
> Cc: dev@dpdk.org; thomas@monjalon.net; gavin.hu@arm.com;
> Honnappa.Nagarahalli@arm.com
> Subject: [EXT] [PATCH 2/6] meson: change default cache line size for cortex-a72
> 
> ----------------------------------------------------------------------
> Per the email discussion [1], the default cache line size of armv8
> cortex-a72 is changed to 64 bytes.

IMO, In git commit you remove the reference to specific discussion and
Update the reason correctly.


> 
> [1] https://mails.dpdk.org/archives/dev/2019-January/123218.html
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>  config/arm/meson.build | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> e00b894523..73c581948c 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -51,6 +51,8 @@ flags_dpaa2 = [
>  	['RTE_MAX_LCORE', 16],
>  	['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]]  flags_default_extra = []
> +flags_cortex_a72_extra = [
> +	['RTE_CACHE_LINE_SIZE', 64]]
>  flags_thunderx_extra = [
>  	['RTE_MACHINE', '"thunderx"'],
>  	['RTE_USE_C11_MEM_MODEL', false]]
> @@ -73,7 +75,7 @@ machine_args_generic = [
>  	['0xd03', ['-mcpu=cortex-a53']],
>  	['0xd04', ['-mcpu=cortex-a35']],
>  	['0xd07', ['-mcpu=cortex-a57']],
> -	['0xd08', ['-mcpu=cortex-a72']],
> +	['0xd08', ['-mcpu=cortex-a72'], flags_cortex_a72_extra],
>  	['0xd09', ['-mcpu=cortex-a73']],
>  	['0xd0a', ['-mcpu=cortex-a75']]]

I think, flags_cortex_a72_extra() can be changed to flags_vendor_arm_extra or something similar
And update the following CPUs also not just cortex-a72.

	['0xd03', ['-mcpu=cortex-a53']],
	['0xd04', ['-mcpu=cortex-a35']],
	['0xd05', ['-mcpu=cortex-a55']],
	['0xd07', ['-mcpu=cortex-a57']],
	['0xd08', ['-mcpu=cortex-a72']],
	['0xd09', ['-mcpu=cortex-a73']],
	['0xd0a', ['-mcpu=cortex-a75']],
	['0xd0b', ['-mcpu=cortex-a76']],


> 
> --
> 2.21.0.196.g041f5ea

Re: [dpdk-dev] [EXT] [PATCH 2/6] meson: change default cache line size for cortex-a72

[Honnappa Nagarahalli]

> >
> > ----------------------------------------------------------------------
> > Per the email discussion [1], the default cache line size of armv8
> > cortex-a72 is changed to 64 bytes.
> 
> IMO, In git commit you remove the reference to specific discussion and
> Update the reason correctly.
> 
> 
> >
> > [1] https://mails.dpdk.org/archives/dev/2019-January/123218.html
> >
> > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > ---
> >  config/arm/meson.build | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > e00b894523..73c581948c 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -51,6 +51,8 @@ flags_dpaa2 = [
> >  	['RTE_MAX_LCORE', 16],
> >  	['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]]  flags_default_extra
> = []
> > +flags_cortex_a72_extra = [
> > +	['RTE_CACHE_LINE_SIZE', 64]]
> >  flags_thunderx_extra = [
Which tree does this patch apply to? I do not see the above line in master.

> >  	['RTE_MACHINE', '"thunderx"'],
> >  	['RTE_USE_C11_MEM_MODEL', false]]
> > @@ -73,7 +75,7 @@ machine_args_generic = [
> >  	['0xd03', ['-mcpu=cortex-a53']],
> >  	['0xd04', ['-mcpu=cortex-a35']],
> >  	['0xd07', ['-mcpu=cortex-a57']],
> > -	['0xd08', ['-mcpu=cortex-a72']],
> > +	['0xd08', ['-mcpu=cortex-a72'], flags_cortex_a72_extra],
> >  	['0xd09', ['-mcpu=cortex-a73']],
> >  	['0xd0a', ['-mcpu=cortex-a75']]]
> 
> I think, flags_cortex_a72_extra() can be changed to
> flags_vendor_arm_extra or something similar And update the following
> CPUs also not just cortex-a72.
> 
Why not add 'flags_arm' similar to flags_dpaa2/flag_cavium etc? All the listed Arm cores are 64B cache line size.

> 	['0xd03', ['-mcpu=cortex-a53']],
> 	['0xd04', ['-mcpu=cortex-a35']],
> 	['0xd05', ['-mcpu=cortex-a55']],
> 	['0xd07', ['-mcpu=cortex-a57']],
> 	['0xd08', ['-mcpu=cortex-a72']],
> 	['0xd09', ['-mcpu=cortex-a73']],
> 	['0xd0a', ['-mcpu=cortex-a75']],
> 	['0xd0b', ['-mcpu=cortex-a76']],
> 
> 
> >
> > --
> > 2.21.0.196.g041f5ea

Re: [dpdk-dev] [EXT] [PATCH 2/6] meson: change default cache line size for cortex-a72

[Honnappa Nagarahalli]

> 
> > >
> > > --------------------------------------------------------------------
> > > -- Per the email discussion [1], the default cache line size of
> > > armv8
> > > cortex-a72 is changed to 64 bytes.
> >
> > IMO, In git commit you remove the reference to specific discussion and
> > Update the reason correctly.
> >
> >
> > >
> > > [1] https://mails.dpdk.org/archives/dev/2019-January/123218.html
> > >
> > > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > > ---
> > >  config/arm/meson.build | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > > e00b894523..73c581948c 100644
> > > --- a/config/arm/meson.build
> > > +++ b/config/arm/meson.build
> > > @@ -51,6 +51,8 @@ flags_dpaa2 = [
> > >  	['RTE_MAX_LCORE', 16],
> > >  	['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]]  flags_default_extra
> > = []
> > > +flags_cortex_a72_extra = [
> > > +	['RTE_CACHE_LINE_SIZE', 64]]
> > >  flags_thunderx_extra = [
> Which tree does this patch apply to? I do not see the above line in master.
Please ignore this comment, I missed the dependency provided in 0/6

> 
> > >  	['RTE_MACHINE', '"thunderx"'],
> > >  	['RTE_USE_C11_MEM_MODEL', false]]
> > > @@ -73,7 +75,7 @@ machine_args_generic = [
> > >  	['0xd03', ['-mcpu=cortex-a53']],
> > >  	['0xd04', ['-mcpu=cortex-a35']],
> > >  	['0xd07', ['-mcpu=cortex-a57']],
> > > -	['0xd08', ['-mcpu=cortex-a72']],
> > > +	['0xd08', ['-mcpu=cortex-a72'], flags_cortex_a72_extra],
> > >  	['0xd09', ['-mcpu=cortex-a73']],
> > >  	['0xd0a', ['-mcpu=cortex-a75']]]
> >
> > I think, flags_cortex_a72_extra() can be changed to
> > flags_vendor_arm_extra or something similar And update the following
> > CPUs also not just cortex-a72.
> >
> Why not add 'flags_arm' similar to flags_dpaa2/flag_cavium etc? All the
> listed Arm cores are 64B cache line size.
Just to complete the thought, impl_0x41 can use 'flags_arm' instead of 'flags_generic'. IMO, current use of 'flags_generic' in impl_0x41 is incorrect.

> 
> > 	['0xd03', ['-mcpu=cortex-a53']],
> > 	['0xd04', ['-mcpu=cortex-a35']],
> > 	['0xd05', ['-mcpu=cortex-a55']],
> > 	['0xd07', ['-mcpu=cortex-a57']],
> > 	['0xd08', ['-mcpu=cortex-a72']],
> > 	['0xd09', ['-mcpu=cortex-a73']],
> > 	['0xd0a', ['-mcpu=cortex-a75']],
> > 	['0xd0b', ['-mcpu=cortex-a76']],
> >
> >
> > >
> > > --
> > > 2.21.0.196.g041f5ea

Re: [dpdk-dev] [EXT] [PATCH 2/6] meson: change default cache line size for cortex-a72

[Yongseok Koh]

> On Apr 15, 2019, at 6:40 AM, Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
> 
>> 
>>>> 
>>>> --------------------------------------------------------------------
>>>> -- Per the email discussion [1], the default cache line size of
>>>> armv8
>>>> cortex-a72 is changed to 64 bytes.
>>> 
>>> IMO, In git commit you remove the reference to specific discussion and
>>> Update the reason correctly.
>>> 
>>> 
>>>> 
>>>> [1] https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.dpdk.org%2Farchives%2Fdev%2F2019-January%2F123218.html&amp;data=02%7C01%7Cyskoh%40mellanox.com%7C4c0cdd9535c84c8dd3c008d6c1a7f5eb%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636909324474698429&amp;sdata=UJO2lBtnYWSs5ud8CsAL7oGXH571f6zGjrVmP2SRChw%3D&amp;reserved=0
>>>> 
>>>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>>>> ---
>>>> config/arm/meson.build | 4 +++-
>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>> 
>>>> diff --git a/config/arm/meson.build b/config/arm/meson.build index
>>>> e00b894523..73c581948c 100644
>>>> --- a/config/arm/meson.build
>>>> +++ b/config/arm/meson.build
>>>> @@ -51,6 +51,8 @@ flags_dpaa2 = [
>>>> 	['RTE_MAX_LCORE', 16],
>>>> 	['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]]  flags_default_extra
>>> = []
>>>> +flags_cortex_a72_extra = [
>>>> +	['RTE_CACHE_LINE_SIZE', 64]]
>>>> flags_thunderx_extra = [
>> Which tree does this patch apply to? I do not see the above line in master.
> Please ignore this comment, I missed the dependency provided in 0/6
> 
>> 
>>>> 	['RTE_MACHINE', '"thunderx"'],
>>>> 	['RTE_USE_C11_MEM_MODEL', false]]
>>>> @@ -73,7 +75,7 @@ machine_args_generic = [
>>>> 	['0xd03', ['-mcpu=cortex-a53']],
>>>> 	['0xd04', ['-mcpu=cortex-a35']],
>>>> 	['0xd07', ['-mcpu=cortex-a57']],
>>>> -	['0xd08', ['-mcpu=cortex-a72']],
>>>> +	['0xd08', ['-mcpu=cortex-a72'], flags_cortex_a72_extra],
>>>> 	['0xd09', ['-mcpu=cortex-a73']],
>>>> 	['0xd0a', ['-mcpu=cortex-a75']]]
>>> 
>>> I think, flags_cortex_a72_extra() can be changed to
>>> flags_vendor_arm_extra or something similar And update the following
>>> CPUs also not just cortex-a72.
>>> 
>> Why not add 'flags_arm' similar to flags_dpaa2/flag_cavium etc? All the
>> listed Arm cores are 64B cache line size.

If so, I'd take your approach - flags_arm.
If we have an exception (CL size is 128 for some cpu) someday,
then we can add an extra flag for that.

> Just to complete the thought, impl_0x41 can use 'flags_arm' instead of 'flags_generic'. IMO, current use of 'flags_generic' in impl_0x41 is incorrect.
> 
>> 
>>> 	['0xd03', ['-mcpu=cortex-a53']],
>>> 	['0xd04', ['-mcpu=cortex-a35']],
>>> 	['0xd05', ['-mcpu=cortex-a55']],
>>> 	['0xd07', ['-mcpu=cortex-a57']],
>>> 	['0xd08', ['-mcpu=cortex-a72']],
>>> 	['0xd09', ['-mcpu=cortex-a73']],
>>> 	['0xd0a', ['-mcpu=cortex-a75']],
>>> 	['0xd0b', ['-mcpu=cortex-a76']],
>>> 
>>> 
>>>> 
>>>> --
>>>> 2.21.0.196.g041f5ea
> 

Re: [dpdk-dev] [EXT] [PATCH 2/6] meson: change default cache line size for cortex-a72

[Honnappa Nagarahalli]

> >
> >>
> >>>>
> >>>> -------------------------------------------------------------------
> >>>> -
> >>>> -- Per the email discussion [1], the default cache line size of
> >>>> armv8
> >>>> cortex-a72 is changed to 64 bytes.
> >>>
> >>> IMO, In git commit you remove the reference to specific discussion
> >>> and Update the reason correctly.
> >>>
> >>>
> >>>>
> >>>> [1]
> >>>>
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fm
> >>>> ails.dpdk.org%2Farchives%2Fdev%2F2019-
> January%2F123218.html&dat
> >>>>
> a=02%7C01%7Cyskoh%40mellanox.com%7C4c0cdd9535c84c8dd3c008d6c1a
> 7f5eb
> >>>> %7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C6369093244
> 74698429&am
> >>>>
> p;sdata=UJO2lBtnYWSs5ud8CsAL7oGXH571f6zGjrVmP2SRChw%3D&re
> served
> >>>> =0
> >>>>
> >>>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> >>>> ---
> >>>> config/arm/meson.build | 4 +++-
> >>>> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> >>>> e00b894523..73c581948c 100644
> >>>> --- a/config/arm/meson.build
> >>>> +++ b/config/arm/meson.build
> >>>> @@ -51,6 +51,8 @@ flags_dpaa2 = [
> >>>> 	['RTE_MAX_LCORE', 16],
> >>>> 	['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]]  flags_default_extra
> >>> = []
> >>>> +flags_cortex_a72_extra = [
> >>>> +	['RTE_CACHE_LINE_SIZE', 64]]
> >>>> flags_thunderx_extra = [
> >> Which tree does this patch apply to? I do not see the above line in
> master.
> > Please ignore this comment, I missed the dependency provided in 0/6
> >
> >>
> >>>> 	['RTE_MACHINE', '"thunderx"'],
> >>>> 	['RTE_USE_C11_MEM_MODEL', false]]
> >>>> @@ -73,7 +75,7 @@ machine_args_generic = [
> >>>> 	['0xd03', ['-mcpu=cortex-a53']],
> >>>> 	['0xd04', ['-mcpu=cortex-a35']],
> >>>> 	['0xd07', ['-mcpu=cortex-a57']],
> >>>> -	['0xd08', ['-mcpu=cortex-a72']],
> >>>> +	['0xd08', ['-mcpu=cortex-a72'], flags_cortex_a72_extra],
> >>>> 	['0xd09', ['-mcpu=cortex-a73']],
> >>>> 	['0xd0a', ['-mcpu=cortex-a75']]]
> >>>
> >>> I think, flags_cortex_a72_extra() can be changed to
> >>> flags_vendor_arm_extra or something similar And update the
> following
> >>> CPUs also not just cortex-a72.
> >>>
> >> Why not add 'flags_arm' similar to flags_dpaa2/flag_cavium etc? All
> >> the listed Arm cores are 64B cache line size.
> 
> If so, I'd take your approach - flags_arm.
> If we have an exception (CL size is 128 for some cpu) someday, then we
> can add an extra flag for that.
> 
Agree. I see the likelihood to be slim given the list of CPUs with 64B

> > Just to complete the thought, impl_0x41 can use 'flags_arm' instead of
> 'flags_generic'. IMO, current use of 'flags_generic' in impl_0x41 is incorrect.
> >
> >>
> >>> 	['0xd03', ['-mcpu=cortex-a53']],
> >>> 	['0xd04', ['-mcpu=cortex-a35']],
> >>> 	['0xd05', ['-mcpu=cortex-a55']],
> >>> 	['0xd07', ['-mcpu=cortex-a57']],
> >>> 	['0xd08', ['-mcpu=cortex-a72']],
> >>> 	['0xd09', ['-mcpu=cortex-a73']],
> >>> 	['0xd0a', ['-mcpu=cortex-a75']],
> >>> 	['0xd0b', ['-mcpu=cortex-a76']],
> >>>
> >>>
> >>>>
> >>>> --
> >>>> 2.21.0.196.g041f5ea
> >

[dpdk-dev] [PATCH 3/6] net/mlx: fix library search in meson build

[Yongseok Koh]

If MLNX_OFED is installed, there's no .pc file installed for libraries and
dependency() can't find libraries by pkg-config. By adding fallback of
using cc.find_library(), libraries are properly located.

Fixes: e30b4e566f47 ("build: improve dependency handling")
Cc: bluca@debian.org
Cc: stable@dpdk.org

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx4/meson.build | 19 +++++++++++--------
 drivers/net/mlx5/meson.build | 19 +++++++++++--------
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/net/mlx4/meson.build b/drivers/net/mlx4/meson.build
index de020701d1..9082f69f25 100644
--- a/drivers/net/mlx4/meson.build
+++ b/drivers/net/mlx4/meson.build
@@ -13,21 +13,24 @@ if pmd_dlopen
 		'-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
 	]
 endif
-libs = [
-	dependency('libmnl', required:false),
-	dependency('libmlx4', required:false),
-	dependency('libibverbs', required:false),
-]
+libs = [ 'libmnl', 'libmlx4', 'libibverbs' ]
+lib_deps = []
 build = true
 foreach lib:libs
-	if not lib.found()
+	lib_dep = dependency(lib, required:false)
+	if not lib_dep.found()
+		lib_dep = cc.find_library(lib, required:false)
+	endif
+	if lib_dep.found()
+		lib_deps += [ lib_dep ]
+	else
 		build = false
 	endif
 endforeach
 # Compile PMD
 if build
 	allow_experimental_apis = true
-	ext_deps += libs
+	ext_deps += lib_deps
 	sources = files(
 		'mlx4.c',
 		'mlx4_ethdev.c',
@@ -103,7 +106,7 @@ if pmd_dlopen and build
 		dlopen_sources,
 		include_directories: global_inc,
 		c_args: cflags,
-		dependencies: libs,
+		dependencies: libs_deps,
 		link_args: [
 		'-Wl,-export-dynamic',
 		'-Wl,-h,@0@'.format(LIB_GLUE),
diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
index a4c684e1b5..42701c51de 100644
--- a/drivers/net/mlx5/meson.build
+++ b/drivers/net/mlx5/meson.build
@@ -13,20 +13,23 @@ if pmd_dlopen
 		'-DMLX5_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
 	]
 endif
-libs = [
-	dependency('libmnl', required:false),
-	dependency('libmlx5', required:false),
-	dependency('libibverbs', required:false),
-]
+libs = [ 'libmnl', 'libmlx5', 'libibverbs' ]
+lib_deps = []
 build = true
 foreach lib:libs
-	if not lib.found()
+	lib_dep = dependency(lib, required:false)
+	if not lib_dep.found()
+		lib_dep = cc.find_library(lib, required:false)
+	endif
+	if lib_dep.found()
+		lib_deps += [ lib_dep ]
+	else
 		build = false
 	endif
 endforeach
 if build
 	allow_experimental_apis = true
-	ext_deps += libs
+	ext_deps += lib_deps
 	sources = files(
 		'mlx5.c',
 		'mlx5_ethdev.c',
@@ -299,7 +302,7 @@ if pmd_dlopen and build
 		dlopen_sources,
 		include_directories: global_inc,
 		c_args: cflags,
-		dependencies: libs,
+		dependencies: lib_deps,
 		link_args: [
 		'-Wl,-export-dynamic',
 		'-Wl,-h,@0@'.format(LIB_GLUE),
-- 
2.21.0.196.g041f5ea

Re: [dpdk-dev] [PATCH 3/6] net/mlx: fix library search in meson build

[Bruce Richardson]

On Fri, Apr 12, 2019 at 04:24:48PM -0700, Yongseok Koh wrote:
> If MLNX_OFED is installed, there's no .pc file installed for libraries and
> dependency() can't find libraries by pkg-config. By adding fallback of
> using cc.find_library(), libraries are properly located.
> 
> Fixes: e30b4e566f47 ("build: improve dependency handling")
> Cc: bluca@debian.org
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>  drivers/net/mlx4/meson.build | 19 +++++++++++--------
>  drivers/net/mlx5/meson.build | 19 +++++++++++--------
>  2 files changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/mlx4/meson.build b/drivers/net/mlx4/meson.build
> index de020701d1..9082f69f25 100644
> --- a/drivers/net/mlx4/meson.build
> +++ b/drivers/net/mlx4/meson.build
> @@ -13,21 +13,24 @@ if pmd_dlopen
>  		'-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
>  	]
>  endif
> -libs = [
> -	dependency('libmnl', required:false),
> -	dependency('libmlx4', required:false),
> -	dependency('libibverbs', required:false),
> -]
> +libs = [ 'libmnl', 'libmlx4', 'libibverbs' ]
> +lib_deps = []

Minor suggestion - you can reduce the size of the diff in this patch by
defining the first array as "libnames" and keeping the actual dependency
objects as "libs".

/Bruce

Re: [dpdk-dev] [PATCH 3/6] net/mlx: fix library search in meson build

[Yongseok Koh]

> On Apr 15, 2019, at 2:19 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> On Fri, Apr 12, 2019 at 04:24:48PM -0700, Yongseok Koh wrote:
>> If MLNX_OFED is installed, there's no .pc file installed for libraries and
>> dependency() can't find libraries by pkg-config. By adding fallback of
>> using cc.find_library(), libraries are properly located.
>> 
>> Fixes: e30b4e566f47 ("build: improve dependency handling")
>> Cc: bluca@debian.org
>> Cc: stable@dpdk.org
>> 
>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>> ---
>> drivers/net/mlx4/meson.build | 19 +++++++++++--------
>> drivers/net/mlx5/meson.build | 19 +++++++++++--------
>> 2 files changed, 22 insertions(+), 16 deletions(-)
>> 
>> diff --git a/drivers/net/mlx4/meson.build b/drivers/net/mlx4/meson.build
>> index de020701d1..9082f69f25 100644
>> --- a/drivers/net/mlx4/meson.build
>> +++ b/drivers/net/mlx4/meson.build
>> @@ -13,21 +13,24 @@ if pmd_dlopen
>> 		'-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
>> 	]
>> endif
>> -libs = [
>> -	dependency('libmnl', required:false),
>> -	dependency('libmlx4', required:false),
>> -	dependency('libibverbs', required:false),
>> -]
>> +libs = [ 'libmnl', 'libmlx4', 'libibverbs' ]
>> +lib_deps = []
> 
> Minor suggestion - you can reduce the size of the diff in this patch by
> defining the first array as "libnames" and keeping the actual dependency
> objects as "libs".

Sounds good to me.
Will take the suggestion in my v2.

Re: [dpdk-dev] [PATCH 3/6] net/mlx: fix library search in meson build

[Luca Boccassi]

On Fri, 2019-04-12 at 16:24 -0700, Yongseok Koh wrote:
> If MLNX_OFED is installed, there's no .pc file installed for
> libraries and
> dependency() can't find libraries by pkg-config. By adding fallback
> of
> using cc.find_library(), libraries are properly located.
> 
> Fixes: e30b4e566f47 ("build: improve dependency handling")
> Cc: 
> bluca@debian.org
> 
> Cc: 
> stable@dpdk.org
> 
> 
> Signed-off-by: Yongseok Koh <
> yskoh@mellanox.com
> >
> ---
>  drivers/net/mlx4/meson.build | 19 +++++++++++--------
>  drivers/net/mlx5/meson.build | 19 +++++++++++--------
>  2 files changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/mlx4/meson.build
> b/drivers/net/mlx4/meson.build
> index de020701d1..9082f69f25 100644
> --- a/drivers/net/mlx4/meson.build
> +++ b/drivers/net/mlx4/meson.build
> @@ -13,21 +13,24 @@ if pmd_dlopen
>  		'-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
>  	]
>  endif
> -libs = [
> -	dependency('libmnl', required:false),
> -	dependency('libmlx4', required:false),
> -	dependency('libibverbs', required:false),
> -]
> +libs = [ 'libmnl', 'libmlx4', 'libibverbs' ]
> +lib_deps = []
>  build = true
>  foreach lib:libs
> -	if not lib.found()
> +	lib_dep = dependency(lib, required:false)
> +	if not lib_dep.found()
> +		lib_dep = cc.find_library(lib, required:false)

Doesn't this end up trying to link the test program to -llibmnl and
thus failing?

> +	endif
> +	if lib_dep.found()
> +		lib_deps += [ lib_dep ]
> +	else
>  		build = false
>  	endif
>  endforeach
>  # Compile PMD
>  if build
>  	allow_experimental_apis = true
> -	ext_deps += libs
> +	ext_deps += lib_deps
>  	sources = files(
>  		'mlx4.c',
>  		'mlx4_ethdev.c',
> @@ -103,7 +106,7 @@ if pmd_dlopen and build
>  		dlopen_sources,
>  		include_directories: global_inc,
>  		c_args: cflags,
> -		dependencies: libs,
> +		dependencies: libs_deps,
>  		link_args: [
>  		'-Wl,-export-dynamic',
>  		'-Wl,-h,@0@'.format(LIB_GLUE),

-- 
Kind regards,
Luca Boccassi

Re: [dpdk-dev] [PATCH 3/6] net/mlx: fix library search in meson build

[Yongseok Koh]

Hi,



Thanks,
Yongseok

> On Apr 15, 2019, at 3:12 AM, Luca Boccassi <bluca@debian.org> wrote:
> 
> On Fri, 2019-04-12 at 16:24 -0700, Yongseok Koh wrote:
>> If MLNX_OFED is installed, there's no .pc file installed for
>> libraries and
>> dependency() can't find libraries by pkg-config. By adding fallback
>> of
>> using cc.find_library(), libraries are properly located.
>> 
>> Fixes: e30b4e566f47 ("build: improve dependency handling")
>> Cc: 
>> bluca@debian.org
>> 
>> Cc: 
>> stable@dpdk.org
>> 
>> 
>> Signed-off-by: Yongseok Koh <
>> yskoh@mellanox.com
>>> 
>> ---
>> drivers/net/mlx4/meson.build | 19 +++++++++++--------
>> drivers/net/mlx5/meson.build | 19 +++++++++++--------
>> 2 files changed, 22 insertions(+), 16 deletions(-)
>> 
>> diff --git a/drivers/net/mlx4/meson.build
>> b/drivers/net/mlx4/meson.build
>> index de020701d1..9082f69f25 100644
>> --- a/drivers/net/mlx4/meson.build
>> +++ b/drivers/net/mlx4/meson.build
>> @@ -13,21 +13,24 @@ if pmd_dlopen
>> 		'-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
>> 	]
>> endif
>> -libs = [
>> -	dependency('libmnl', required:false),
>> -	dependency('libmlx4', required:false),
>> -	dependency('libibverbs', required:false),
>> -]
>> +libs = [ 'libmnl', 'libmlx4', 'libibverbs' ]
>> +lib_deps = []
>> build = true
>> foreach lib:libs
>> -	if not lib.found()
>> +	lib_dep = dependency(lib, required:false)
>> +	if not lib_dep.found()
>> +		lib_dep = cc.find_library(lib, required:false)
> 
> Doesn't this end up trying to link the test program to -llibmnl and
> thus failing?

I also worried about that. But it works fine.
Looks meson is smart enough. :-)

>> +	endif
>> +	if lib_dep.found()
>> +		lib_deps += [ lib_dep ]
>> +	else
>> 		build = false
>> 	endif
>> endforeach
>> # Compile PMD
>> if build
>> 	allow_experimental_apis = true
>> -	ext_deps += libs
>> +	ext_deps += lib_deps
>> 	sources = files(
>> 		'mlx4.c',
>> 		'mlx4_ethdev.c',
>> @@ -103,7 +106,7 @@ if pmd_dlopen and build
>> 		dlopen_sources,
>> 		include_directories: global_inc,
>> 		c_args: cflags,
>> -		dependencies: libs,
>> +		dependencies: libs_deps,
>> 		link_args: [
>> 		'-Wl,-export-dynamic',
>> 		'-Wl,-h,@0@'.format(LIB_GLUE),
> 
> -- 
> Kind regards,
> Luca Boccassi

Re: [dpdk-dev] [PATCH 3/6] net/mlx: fix library search in meson build

[Luca Boccassi]

On Mon, 2019-04-15 at 19:48 +0000, Yongseok Koh wrote:
> Hi,
> 
> 
> 
> Thanks,
> Yongseok
> 
> > On Apr 15, 2019, at 3:12 AM, Luca Boccassi <
> > bluca@debian.org
> > > wrote:
> > 
> > On Fri, 2019-04-12 at 16:24 -0700, Yongseok Koh wrote:
> > > If MLNX_OFED is installed, there's no .pc file installed for
> > > libraries and
> > > dependency() can't find libraries by pkg-config. By adding
> > > fallback
> > > of
> > > using cc.find_library(), libraries are properly located.
> > > 
> > > Fixes: e30b4e566f47 ("build: improve dependency handling")
> > > Cc: 
> > > bluca@debian.org
> > > 
> > > 
> > > Cc: 
> > > stable@dpdk.org
> > > 
> > > 
> > > 
> > > Signed-off-by: Yongseok Koh <
> > > yskoh@mellanox.com
> > > 
> > > 
> > > ---
> > > drivers/net/mlx4/meson.build | 19 +++++++++++--------
> > > drivers/net/mlx5/meson.build | 19 +++++++++++--------
> > > 2 files changed, 22 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/drivers/net/mlx4/meson.build
> > > b/drivers/net/mlx4/meson.build
> > > index de020701d1..9082f69f25 100644
> > > --- a/drivers/net/mlx4/meson.build
> > > +++ b/drivers/net/mlx4/meson.build
> > > @@ -13,21 +13,24 @@ if pmd_dlopen
> > > 		'-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
> > > 	]
> > > endif
> > > -libs = [
> > > -	dependency('libmnl', required:false),
> > > -	dependency('libmlx4', required:false),
> > > -	dependency('libibverbs', required:false),
> > > -]
> > > +libs = [ 'libmnl', 'libmlx4', 'libibverbs' ]
> > > +lib_deps = []
> > > build = true
> > > foreach lib:libs
> > > -	if not lib.found()
> > > +	lib_dep = dependency(lib, required:false)
> > > +	if not lib_dep.found()
> > > +		lib_dep = cc.find_library(lib, required:false)
> > 
> > Doesn't this end up trying to link the test program to -llibmnl and
> > thus failing?
> 
> I also worried about that. But it works fine.
> Looks meson is smart enough. :-)

Sorry, not to be skeptical, but at least with the meson version I was
using when doing something similar I'm sure this didn't work -
find_library just takes the parameter, adds "-l" in front of it and
uses it to compile.

In the meson configure log, do you see:

Dependency libmlx4 found: NO
Library libmlx4 found: YES

?

-- 
Kind regards,
Luca Boccassi

Re: [dpdk-dev] [PATCH 3/6] net/mlx: fix library search in meson build

[Bruce Richardson]

On Thu, Apr 18, 2019 at 10:25:19AM +0100, Luca Boccassi wrote:
> On Mon, 2019-04-15 at 19:48 +0000, Yongseok Koh wrote:
> > Hi,
> > 
> > 
> > 
> > Thanks,
> > Yongseok
> > 
> > > On Apr 15, 2019, at 3:12 AM, Luca Boccassi <
> > > bluca@debian.org
> > > > wrote:
> > > 
> > > On Fri, 2019-04-12 at 16:24 -0700, Yongseok Koh wrote:
> > > > If MLNX_OFED is installed, there's no .pc file installed for
> > > > libraries and
> > > > dependency() can't find libraries by pkg-config. By adding
> > > > fallback
> > > > of
> > > > using cc.find_library(), libraries are properly located.
> > > > 
> > > > Fixes: e30b4e566f47 ("build: improve dependency handling")
> > > > Cc: 
> > > > bluca@debian.org
> > > > 
> > > > 
> > > > Cc: 
> > > > stable@dpdk.org
> > > > 
> > > > 
> > > > 
> > > > Signed-off-by: Yongseok Koh <
> > > > yskoh@mellanox.com
> > > > 
> > > > 
> > > > ---
> > > > drivers/net/mlx4/meson.build | 19 +++++++++++--------
> > > > drivers/net/mlx5/meson.build | 19 +++++++++++--------
> > > > 2 files changed, 22 insertions(+), 16 deletions(-)
> > > > 
> > > > diff --git a/drivers/net/mlx4/meson.build
> > > > b/drivers/net/mlx4/meson.build
> > > > index de020701d1..9082f69f25 100644
> > > > --- a/drivers/net/mlx4/meson.build
> > > > +++ b/drivers/net/mlx4/meson.build
> > > > @@ -13,21 +13,24 @@ if pmd_dlopen
> > > > 		'-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
> > > > 	]
> > > > endif
> > > > -libs = [
> > > > -	dependency('libmnl', required:false),
> > > > -	dependency('libmlx4', required:false),
> > > > -	dependency('libibverbs', required:false),
> > > > -]
> > > > +libs = [ 'libmnl', 'libmlx4', 'libibverbs' ]
> > > > +lib_deps = []
> > > > build = true
> > > > foreach lib:libs
> > > > -	if not lib.found()
> > > > +	lib_dep = dependency(lib, required:false)
> > > > +	if not lib_dep.found()
> > > > +		lib_dep = cc.find_library(lib, required:false)
> > > 
> > > Doesn't this end up trying to link the test program to -llibmnl and
> > > thus failing?
> > 
> > I also worried about that. But it works fine.
> > Looks meson is smart enough. :-)
> 
> Sorry, not to be skeptical, but at least with the meson version I was
> using when doing something similar I'm sure this didn't work -
> find_library just takes the parameter, adds "-l" in front of it and
> uses it to compile.
> 
> In the meson configure log, do you see:
> 
> Dependency libmlx4 found: NO
> Library libmlx4 found: YES
> 

My understanding was the same as yours, Luca, and I'm pretty sure older
versions used to have that restriction. I think we should - out of an
abundance of caution - remove the "lib" prefix from all library/dependency
requests.

/Bruce

Re: [dpdk-dev] [PATCH 3/6] net/mlx: fix library search in meson build

[Yongseok Koh]

> On Apr 18, 2019, at 3:14 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> On Thu, Apr 18, 2019 at 10:25:19AM +0100, Luca Boccassi wrote:
>> On Mon, 2019-04-15 at 19:48 +0000, Yongseok Koh wrote:
>>> Hi,
>>> 
>>> 
>>> 
>>> Thanks,
>>> Yongseok
>>> 
>>>> On Apr 15, 2019, at 3:12 AM, Luca Boccassi <
>>>> bluca@debian.org
>>>>> wrote:
>>>> 
>>>> On Fri, 2019-04-12 at 16:24 -0700, Yongseok Koh wrote:
>>>>> If MLNX_OFED is installed, there's no .pc file installed for
>>>>> libraries and
>>>>> dependency() can't find libraries by pkg-config. By adding
>>>>> fallback
>>>>> of
>>>>> using cc.find_library(), libraries are properly located.
>>>>> 
>>>>> Fixes: e30b4e566f47 ("build: improve dependency handling")
>>>>> Cc: 
>>>>> bluca@debian.org
>>>>> 
>>>>> 
>>>>> Cc: 
>>>>> stable@dpdk.org
>>>>> 
>>>>> 
>>>>> 
>>>>> Signed-off-by: Yongseok Koh <
>>>>> yskoh@mellanox.com
>>>>> 
>>>>> 
>>>>> ---
>>>>> drivers/net/mlx4/meson.build | 19 +++++++++++--------
>>>>> drivers/net/mlx5/meson.build | 19 +++++++++++--------
>>>>> 2 files changed, 22 insertions(+), 16 deletions(-)
>>>>> 
>>>>> diff --git a/drivers/net/mlx4/meson.build
>>>>> b/drivers/net/mlx4/meson.build
>>>>> index de020701d1..9082f69f25 100644
>>>>> --- a/drivers/net/mlx4/meson.build
>>>>> +++ b/drivers/net/mlx4/meson.build
>>>>> @@ -13,21 +13,24 @@ if pmd_dlopen
>>>>> 		'-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
>>>>> 	]
>>>>> endif
>>>>> -libs = [
>>>>> -	dependency('libmnl', required:false),
>>>>> -	dependency('libmlx4', required:false),
>>>>> -	dependency('libibverbs', required:false),
>>>>> -]
>>>>> +libs = [ 'libmnl', 'libmlx4', 'libibverbs' ]
>>>>> +lib_deps = []
>>>>> build = true
>>>>> foreach lib:libs
>>>>> -	if not lib.found()
>>>>> +	lib_dep = dependency(lib, required:false)
>>>>> +	if not lib_dep.found()
>>>>> +		lib_dep = cc.find_library(lib, required:false)
>>>> 
>>>> Doesn't this end up trying to link the test program to -llibmnl and
>>>> thus failing?
>>> 
>>> I also worried about that. But it works fine.
>>> Looks meson is smart enough. :-)
>> 
>> Sorry, not to be skeptical, but at least with the meson version I was
>> using when doing something similar I'm sure this didn't work -
>> find_library just takes the parameter, adds "-l" in front of it and
>> uses it to compile.
>> 
>> In the meson configure log, do you see:
>> 
>> Dependency libmlx4 found: NO
>> Library libmlx4 found: YES

Thanks for the note, Luca.

It worked regardless. Compilation's done successfully and the final testpmd
binary was good to run. Don't know how it worked but will change it anyway. Not
a hard task. Here's the diff.

$ git diff
diff --git a/drivers/net/mlx4/meson.build b/drivers/net/mlx4/meson.build
index 9d04dd930d..2540489bb7 100644
--- a/drivers/net/mlx4/meson.build
+++ b/drivers/net/mlx4/meson.build
@@ -13,11 +13,11 @@ if pmd_dlopen
                '-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
        ]
 endif
-libnames = [ 'libmnl', 'libmlx4', 'libibverbs' ]
+libnames = [ 'mnl', 'mlx4', 'ibverbs' ]
 libs = []
 build = true
 foreach libname:libnames
-       lib = dependency(libname, required:false)
+       lib = dependency('lib' + libname, required:false)
        if not lib.found()
                lib = cc.find_library(libname, required:false)
        endif
diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
index ee8399af27..1a65c3a989 100644
--- a/drivers/net/mlx5/meson.build
+++ b/drivers/net/mlx5/meson.build
@@ -13,11 +13,11 @@ if pmd_dlopen
                '-DMLX5_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
        ]
 endif
-libnames = [ 'libmnl', 'libmlx5', 'libibverbs' ]
+libnames = [ 'mnl', 'mlx5', 'ibverbs' ]
 libs = []
 build = true
 foreach libname:libnames
-       lib = dependency(libname, required:false)
+       lib = dependency('lib' + libname, required:false)
        if not lib.found()
                lib = cc.find_library(libname, required:false)
        endif

Then, it will give us:

Dependency libmnl found: NO
Library mnl found: YES
Dependency libmlx5 found: NO
Library mlx5 found: YES
Dependency libibverbs found: NO
Library ibverbs found: YES


Thanks,
Yongseok

[dpdk-dev] [PATCH 4/6] meson: add Mellanox BlueField cross-compile config

[Yongseok Koh]

Mellanox BlueField is armv8 CPU having cortex-a72. The implementor ID is
0x41 (arm) and the primary part number is 0xd08 (cortex-a72).

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 config/arm/arm64_bluefield_linux_gcc | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 config/arm/arm64_bluefield_linux_gcc

diff --git a/config/arm/arm64_bluefield_linux_gcc b/config/arm/arm64_bluefield_linux_gcc
new file mode 100644
index 0000000000..304c4073d5
--- /dev/null
+++ b/config/arm/arm64_bluefield_linux_gcc
@@ -0,0 +1,16 @@
+[binaries]
+c = 'aarch64-linux-gnu-gcc'
+cpp = 'aarch64-linux-gnu-cpp'
+ar = 'aarch64-linux-gnu-gcc-ar'
+strip = 'aarch64-linux-gnu-strip'
+pcap-config = ''
+
+[host_machine]
+system = 'linux'
+cpu_family = 'aarch64'
+cpu = 'armv8-a'
+endian = 'little'
+
+[properties]
+implementor_id = '0x41'
+implementor_pn = '0xd08'
-- 
2.21.0.196.g041f5ea

Re: [dpdk-dev] [EXT] [PATCH 4/6] meson: add Mellanox BlueField cross-compile config

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Yongseok Koh <yskoh@mellanox.com>
> Sent: Saturday, April 13, 2019 4:55 AM
> To: bruce.richardson@intel.com; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>; shahafs@mellanox.com
> Cc: dev@dpdk.org; thomas@monjalon.net; gavin.hu@arm.com;
> Honnappa.Nagarahalli@arm.com
> Subject: [EXT] [PATCH 4/6] meson: add Mellanox BlueField cross-compile config
> 
> ----------------------------------------------------------------------
> Mellanox BlueField is armv8 CPU having cortex-a72. The implementor ID is
> 0x41 (arm) and the primary part number is 0xd08 (cortex-a72).
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>


Acked-by: Jerin Jacob <jerinj@marvell.com>

[dpdk-dev] [PATCH 5/6] build: add option for armv8 crypto extension

[Yongseok Koh]

Per armv8 crypto extension support, make build always enable it by default
as long as compiler supports the feature while meson build only enables it
for 'default' machine of generic armv8 architecture. For example,
specifying '-mcpu=cortex-a72' doesn't enable it but '+crypto' is required
in order to enable the feature.

It is also known that not all the armv8 platforms have the crypto
extension. For example, Mellanox BlueField has a variant which doesn't have
it. If crypto enabled binary runs on such a platform, rte_eal_init() fails.

Therefore, an option to control this feature is necessary. It is still
enabled by default but can be selectively disabled by vendors.

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 config/arm/meson.build        | 16 +++++++++-------
 config/common_armv8a_linux    |  1 +
 drivers/crypto/armv8/Makefile |  4 ++++
 meson_options.txt             |  2 ++
 mk/machine/armv8a/rte.vars.mk |  4 ++++
 5 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/config/arm/meson.build b/config/arm/meson.build
index 73c581948c..762d222ed5 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -7,6 +7,8 @@ march_opt = '-march=@0@'.format(machine)
 
 arm_force_native_march = false
 
+crypto_flag = get_option('enable_armv8_crypto') ? '+crypto' : ''
+
 flags_common_default = [
 	# Accelarate rte_memcpy. Be sure to run unit test (memcpy_perf_autotest)
 	# to determine the best threshold in code. Refer to notes in source file
@@ -70,14 +72,14 @@ flags_octeontx2_extra = [
 	['RTE_USE_C11_MEM_MODEL', true]]
 
 machine_args_generic = [
-	['default', ['-march=armv8-a+crc+crypto']],
+	['default', ['-march=armv8-a+crc' + crypto_flag]],
 	['native', ['-march=native']],
-	['0xd03', ['-mcpu=cortex-a53']],
-	['0xd04', ['-mcpu=cortex-a35']],
-	['0xd07', ['-mcpu=cortex-a57']],
-	['0xd08', ['-mcpu=cortex-a72'], flags_cortex_a72_extra],
-	['0xd09', ['-mcpu=cortex-a73']],
-	['0xd0a', ['-mcpu=cortex-a75']]]
+	['0xd03', ['-mcpu=cortex-a53' + crypto_flag]],
+	['0xd04', ['-mcpu=cortex-a35' + crypto_flag]],
+	['0xd07', ['-mcpu=cortex-a57' + crypto_flag]],
+	['0xd08', ['-mcpu=cortex-a72' + crypto_flag], flags_cortex_a72_extra],
+	['0xd09', ['-mcpu=cortex-a73' + crypto_flag]],
+	['0xd0a', ['-mcpu=cortex-a75' + crypto_flag]]]
 
 machine_args_cavium = [
 	['default', ['-march=armv8-a+crc+crypto','-mcpu=thunderx']],
diff --git a/config/common_armv8a_linux b/config/common_armv8a_linux
index 72091de1c7..0efa3e2eb2 100644
--- a/config/common_armv8a_linux
+++ b/config/common_armv8a_linux
@@ -5,6 +5,7 @@
 #include "common_linux"
 
 CONFIG_RTE_MACHINE="armv8a"
+CONFIG_RTE_ENABLE_ARMV8_CRYPTO=y
 
 CONFIG_RTE_ARCH="arm64"
 CONFIG_RTE_ARCH_ARM64=y
diff --git a/drivers/crypto/armv8/Makefile b/drivers/crypto/armv8/Makefile
index f71f6b14a4..867a5206cf 100644
--- a/drivers/crypto/armv8/Makefile
+++ b/drivers/crypto/armv8/Makefile
@@ -4,6 +4,10 @@
 
 include $(RTE_SDK)/mk/rte.vars.mk
 
+ifneq ($(CONFIG_RTE_ENABLE_ARMV8_CRYPTO),y)
+$(error "Please enable CONFIG_RTE_ENABLE_ARMV8_CRYPTO")
+endif
+
 ifneq ($(MAKECMDGOALS),clean)
 ifneq ($(MAKECMDGOALS),config)
 ifeq ($(ARMV8_CRYPTO_LIB_PATH),)
diff --git a/meson_options.txt b/meson_options.txt
index 16d9f92c65..4ca09771de 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -4,6 +4,8 @@ option('allow_invalid_socket_id', type: 'boolean', value: false,
 	description: 'allow out-of-range NUMA socket id\'s for platforms that don\'t report the value correctly')
 option('drivers_install_subdir', type: 'string', value: 'dpdk/pmds-<VERSION>',
 	description: 'Subdirectory of libdir where to install PMDs. Defaults to using a versioned subdirectory.')
+option('enable_armv8_crypto', type: 'boolean', value: true,
+	description: 'enable armv8 crypto extension')
 option('enable_docs', type: 'boolean', value: false,
 	description: 'build documentation')
 option('enable_kmods', type: 'boolean', value: true,
diff --git a/mk/machine/armv8a/rte.vars.mk b/mk/machine/armv8a/rte.vars.mk
index 8252efbb7b..4893d01a2d 100644
--- a/mk/machine/armv8a/rte.vars.mk
+++ b/mk/machine/armv8a/rte.vars.mk
@@ -28,4 +28,8 @@
 # CPU_LDFLAGS =
 # CPU_ASFLAGS =
 
+ifeq ($(CONFIG_RTE_ENABLE_ARMV8_CRYPTO),y)
 MACHINE_CFLAGS += -march=armv8-a+crc+crypto
+else
+MACHINE_CFLAGS += -march=armv8-a+crc
+endif
-- 
2.21.0.196.g041f5ea

Re: [dpdk-dev] [EXT] [PATCH 5/6] build: add option for armv8 crypto extension

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Yongseok Koh <yskoh@mellanox.com>
> Sent: Saturday, April 13, 2019 4:55 AM
> To: bruce.richardson@intel.com; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>; shahafs@mellanox.com
> Cc: dev@dpdk.org; thomas@monjalon.net; gavin.hu@arm.com;
> Honnappa.Nagarahalli@arm.com
> Subject: [EXT] [PATCH 5/6] build: add option for armv8 crypto extension
> 
>  CONFIG_RTE_MACHINE="armv8a"
> +CONFIG_RTE_ENABLE_ARMV8_CRYPTO=y

This approach is not scalable. Even, it is not good for BlueField as you 
you need to maintain two images.

Unlike other CPU flags, arm64's crypto cpu flag is really _optional_.
Access to crypto instructions is always at under runtime check.
See the following in rte_armv8_pmd.c


	/* Check CPU for support for AES instruction set */
	if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AES)) {
		ARMV8_CRYPTO_LOG_ERR(
			"AES instructions not supported by CPU");
		return -EFAULT;
	}

	/* Check CPU for support for SHA instruction set */
	if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA1) ||
	    !rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA2)) {
		ARMV8_CRYPTO_LOG_ERR(
			"SHA1/SHA2 instructions not supported by CPU");
		return -EFAULT;
	}

So In order to avoid one more config flags specific to armv8 in meson and makefile build infra
And avoid the need for 6/6 patch. IMO,
# Introduce optional CPU flag scheme in eal. Treat armv8 crypto as optional flag
# Skip the eal init check for optional flag.

Do you see any issues with that approach?

Re: [dpdk-dev] [EXT] [PATCH 5/6] build: add option for armv8 crypto extension

[Honnappa Nagarahalli]

> >
> >  CONFIG_RTE_MACHINE="armv8a"
> > +CONFIG_RTE_ENABLE_ARMV8_CRYPTO=y
> 
> This approach is not scalable. Even, it is not good for BlueField as you you
> need to maintain two images.
> 
> Unlike other CPU flags, arm64's crypto cpu flag is really _optional_.
> Access to crypto instructions is always at under runtime check.
> See the following in rte_armv8_pmd.c
> 
> 
> 	/* Check CPU for support for AES instruction set */
> 	if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AES)) {
> 		ARMV8_CRYPTO_LOG_ERR(
> 			"AES instructions not supported by CPU");
> 		return -EFAULT;
> 	}
> 
> 	/* Check CPU for support for SHA instruction set */
> 	if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA1) ||
> 	    !rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA2)) {
> 		ARMV8_CRYPTO_LOG_ERR(
> 			"SHA1/SHA2 instructions not supported by CPU");
> 		return -EFAULT;
> 	}
> 
> So In order to avoid one more config flags specific to armv8 in meson and
> makefile build infra And avoid the need for 6/6 patch. IMO, # Introduce
> optional CPU flag scheme in eal. Treat armv8 crypto as optional flag # Skip
> the eal init check for optional flag.
> 
> Do you see any issues with that approach?
> 
+1

> 
> 
> 
> 
> 

Re: [dpdk-dev] [EXT] [PATCH 5/6] build: add option for armv8 crypto extension

[Yongseok Koh]

> On Apr 13, 2019, at 12:22 AM, Jerin Jacob Kollanukkaran <jerinj@marvell.com> wrote:
> 
>> -----Original Message-----
>> From: Yongseok Koh <yskoh@mellanox.com>
>> Sent: Saturday, April 13, 2019 4:55 AM
>> To: bruce.richardson@intel.com; Jerin Jacob Kollanukkaran
>> <jerinj@marvell.com>; Pavan Nikhilesh Bhagavatula
>> <pbhagavatula@marvell.com>; shahafs@mellanox.com
>> Cc: dev@dpdk.org; thomas@monjalon.net; gavin.hu@arm.com;
>> Honnappa.Nagarahalli@arm.com
>> Subject: [EXT] [PATCH 5/6] build: add option for armv8 crypto extension
>> 
>> CONFIG_RTE_MACHINE="armv8a"
>> +CONFIG_RTE_ENABLE_ARMV8_CRYPTO=y
> 
> This approach is not scalable. Even, it is not good for BlueField as you 
> you need to maintain two images.
> 
> Unlike other CPU flags, arm64's crypto cpu flag is really _optional_.
> Access to crypto instructions is always at under runtime check.
> See the following in rte_armv8_pmd.c
> 
> 
> 	/* Check CPU for support for AES instruction set */
> 	if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AES)) {
> 		ARMV8_CRYPTO_LOG_ERR(
> 			"AES instructions not supported by CPU");
> 		return -EFAULT;
> 	}
> 
> 	/* Check CPU for support for SHA instruction set */
> 	if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA1) ||
> 	    !rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA2)) {
> 		ARMV8_CRYPTO_LOG_ERR(
> 			"SHA1/SHA2 instructions not supported by CPU");
> 		return -EFAULT;
> 	}
> 
> So In order to avoid one more config flags specific to armv8 in meson and makefile build infra
> And avoid the need for 6/6 patch. IMO,
> # Introduce optional CPU flag scheme in eal. Treat armv8 crypto as optional flag
> # Skip the eal init check for optional flag.
> 
> Do you see any issues with that approach?

I also thought about that approach and that was my number 1 priority. But, I had
one question came to my mind. Maybe, arm people can confirm it. Is it 100%
guaranteed that compiler never makes use of any of crypto instructions even if
there's no specific asm/intrinsic code?  The crypto extension has aes, pmull,
sha1 and sha2. In case of rte_memcpy() for x86, for example, compiler may
optimize code using avx512f instructions even though it is written specifically
with avx2 intrinsics (__mm256_*) unless avx512f is disabled.

If a complier expert in arm (or anyone else) confirm it is completely
**optional**, then I'd love to take that approach for sure.

Copied dpdk-on-arm ML.


Thanks,
Yongseok

Re: [dpdk-dev] [EXT] [PATCH 5/6] build: add option for armv8 crypto extension

[Honnappa Nagarahalli]

> >> Subject: [EXT] [PATCH 5/6] build: add option for armv8 crypto
> >> extension
> >>
> >> CONFIG_RTE_MACHINE="armv8a"
> >> +CONFIG_RTE_ENABLE_ARMV8_CRYPTO=y
> >
> > This approach is not scalable. Even, it is not good for BlueField as
> > you you need to maintain two images.
> >
> > Unlike other CPU flags, arm64's crypto cpu flag is really _optional_.
> > Access to crypto instructions is always at under runtime check.
> > See the following in rte_armv8_pmd.c
> >
> >
> > 	/* Check CPU for support for AES instruction set */
> > 	if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AES)) {
> > 		ARMV8_CRYPTO_LOG_ERR(
> > 			"AES instructions not supported by CPU");
> > 		return -EFAULT;
> > 	}
> >
> > 	/* Check CPU for support for SHA instruction set */
> > 	if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA1) ||
> > 	    !rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA2)) {
> > 		ARMV8_CRYPTO_LOG_ERR(
> > 			"SHA1/SHA2 instructions not supported by CPU");
> > 		return -EFAULT;
> > 	}
> >
> > So In order to avoid one more config flags specific to armv8 in meson
> > and makefile build infra And avoid the need for 6/6 patch. IMO, #
> > Introduce optional CPU flag scheme in eal. Treat armv8 crypto as
> > optional flag # Skip the eal init check for optional flag.
> >
> > Do you see any issues with that approach?
> 
> I also thought about that approach and that was my number 1 priority.
> But, I had one question came to my mind. Maybe, arm people can confirm
> it. Is it 100% guaranteed that compiler never makes use of any of crypto
> instructions even if there's no specific asm/intrinsic code?  The crypto
> extension has aes, pmull,
> sha1 and sha2. In case of rte_memcpy() for x86, for example, compiler may
> optimize code using avx512f instructions even though it is written
> specifically with avx2 intrinsics (__mm256_*) unless avx512f is disabled.
> 
> If a complier expert in arm (or anyone else) confirm it is completely
> **optional**, then I'd love to take that approach for sure.
> 
> Copied dpdk-on-arm ML.
> 
I do not know the answer, will have to check with the compiler team. I will get back on this.

> 
> Thanks,
> Yongseok
> 

Re: [dpdk-dev] [EXT] [PATCH 5/6] build: add option for armv8 crypto extension

[Yongseok Koh]

On Apr 15, 2019, at 1:13 PM, Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:

>>>> Subject: [EXT] [PATCH 5/6] build: add option for armv8 crypto
>>>> extension
>>>> 
>>>> CONFIG_RTE_MACHINE="armv8a"
>>>> +CONFIG_RTE_ENABLE_ARMV8_CRYPTO=y
>>> 
>>> This approach is not scalable. Even, it is not good for BlueField as
>>> you you need to maintain two images.
>>> 
>>> Unlike other CPU flags, arm64's crypto cpu flag is really _optional_.
>>> Access to crypto instructions is always at under runtime check.
>>> See the following in rte_armv8_pmd.c
>>> 
>>> 
>>>    /* Check CPU for support for AES instruction set */
>>>    if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AES)) {
>>>        ARMV8_CRYPTO_LOG_ERR(
>>>            "AES instructions not supported by CPU");
>>>        return -EFAULT;
>>>    }
>>> 
>>>    /* Check CPU for support for SHA instruction set */
>>>    if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA1) ||
>>>        !rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA2)) {
>>>        ARMV8_CRYPTO_LOG_ERR(
>>>            "SHA1/SHA2 instructions not supported by CPU");
>>>        return -EFAULT;
>>>    }
>>> 
>>> So In order to avoid one more config flags specific to armv8 in meson
>>> and makefile build infra And avoid the need for 6/6 patch. IMO, #
>>> Introduce optional CPU flag scheme in eal. Treat armv8 crypto as
>>> optional flag # Skip the eal init check for optional flag.
>>> 
>>> Do you see any issues with that approach?
>> 
>> I also thought about that approach and that was my number 1 priority.
>> But, I had one question came to my mind. Maybe, arm people can confirm
>> it. Is it 100% guaranteed that compiler never makes use of any of crypto
>> instructions even if there's no specific asm/intrinsic code?  The crypto
>> extension has aes, pmull,
>> sha1 and sha2. In case of rte_memcpy() for x86, for example, compiler may
>> optimize code using avx512f instructions even though it is written
>> specifically with avx2 intrinsics (__mm256_*) unless avx512f is disabled.
>> 
>> If a complier expert in arm (or anyone else) confirm it is completely
>> **optional**, then I'd love to take that approach for sure.
>> 
>> Copied dpdk-on-arm ML.
>> 
> I do not know the answer, will have to check with the compiler team. I will get back on this.

Any update yet?

Thanks 
Yongseok 

Re: [dpdk-dev] [EXT] [PATCH 5/6] build: add option for armv8 crypto extension

[Honnappa Nagarahalli]

> On Apr 15, 2019, at 1:13 PM, Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com> wrote:
> 
> >>>> Subject: [EXT] [PATCH 5/6] build: add option for armv8 crypto
> >>>> extension
> >>>>
> >>>> CONFIG_RTE_MACHINE="armv8a"
> >>>> +CONFIG_RTE_ENABLE_ARMV8_CRYPTO=y
> >>>
> >>> This approach is not scalable. Even, it is not good for BlueField as
> >>> you you need to maintain two images.
> >>>
> >>> Unlike other CPU flags, arm64's crypto cpu flag is really _optional_.
> >>> Access to crypto instructions is always at under runtime check.
> >>> See the following in rte_armv8_pmd.c
> >>>
> >>>
> >>>    /* Check CPU for support for AES instruction set */
> >>>    if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AES)) {
> >>>        ARMV8_CRYPTO_LOG_ERR(
> >>>            "AES instructions not supported by CPU");
> >>>        return -EFAULT;
> >>>    }
> >>>
> >>>    /* Check CPU for support for SHA instruction set */
> >>>    if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA1) ||
> >>>        !rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA2)) {
> >>>        ARMV8_CRYPTO_LOG_ERR(
> >>>            "SHA1/SHA2 instructions not supported by CPU");
> >>>        return -EFAULT;
> >>>    }
> >>>
> >>> So In order to avoid one more config flags specific to armv8 in
> >>> meson and makefile build infra And avoid the need for 6/6 patch.
> >>> IMO, # Introduce optional CPU flag scheme in eal. Treat armv8 crypto
> >>> as optional flag # Skip the eal init check for optional flag.
> >>>
> >>> Do you see any issues with that approach?
> >>
> >> I also thought about that approach and that was my number 1 priority.
> >> But, I had one question came to my mind. Maybe, arm people can
> >> confirm it. Is it 100% guaranteed that compiler never makes use of
> >> any of crypto instructions even if there's no specific asm/intrinsic
> >> code?  The crypto extension has aes, pmull,
> >> sha1 and sha2. In case of rte_memcpy() for x86, for example, compiler
> >> may optimize code using avx512f instructions even though it is
> >> written specifically with avx2 intrinsics (__mm256_*) unless avx512f is
> disabled.
> >>
> >> If a complier expert in arm (or anyone else) confirm it is completely
> >> **optional**, then I'd love to take that approach for sure.
> >>
> >> Copied dpdk-on-arm ML.
> >>
> > I do not know the answer, will have to check with the compiler team. I will get
> back on this.
> 
> Any update yet?
Currently, enabling 'crypto' flag will generate the crypto instructions only when crypto intrinsics are used. However, when 'sha3' (part of 8.2 crypto) flag is enabled, compiler can generate 3-way exclusive OR instructions beyond the intrinsics. Compiler team cannot provide a guarantee that other crypto instructions will not be used beyond the intrinsics.

The current suggestion is to use GNU indirect function [1] or similar. I am not sure on GNU indirect function portability.

[1] https://willnewton.name/2013/07/02/using-gnu-indirect-functions/

> 
> Thanks
> Yongseok

Re: [dpdk-dev] [EXT] [PATCH 5/6] build: add option for armv8 crypto extension

[Yongseok Koh]

> On Apr 29, 2019, at 8:33 PM, Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
> 
>> On Apr 15, 2019, at 1:13 PM, Honnappa Nagarahalli
>> <Honnappa.Nagarahalli@arm.com> wrote:
>> 
>>>>>> Subject: [EXT] [PATCH 5/6] build: add option for armv8 crypto
>>>>>> extension
>>>>>> 
>>>>>> CONFIG_RTE_MACHINE="armv8a"
>>>>>> +CONFIG_RTE_ENABLE_ARMV8_CRYPTO=y
>>>>> 
>>>>> This approach is not scalable. Even, it is not good for BlueField as
>>>>> you you need to maintain two images.
>>>>> 
>>>>> Unlike other CPU flags, arm64's crypto cpu flag is really _optional_.
>>>>> Access to crypto instructions is always at under runtime check.
>>>>> See the following in rte_armv8_pmd.c
>>>>> 
>>>>> 
>>>>>   /* Check CPU for support for AES instruction set */
>>>>>   if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AES)) {
>>>>>       ARMV8_CRYPTO_LOG_ERR(
>>>>>           "AES instructions not supported by CPU");
>>>>>       return -EFAULT;
>>>>>   }
>>>>> 
>>>>>   /* Check CPU for support for SHA instruction set */
>>>>>   if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA1) ||
>>>>>       !rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA2)) {
>>>>>       ARMV8_CRYPTO_LOG_ERR(
>>>>>           "SHA1/SHA2 instructions not supported by CPU");
>>>>>       return -EFAULT;
>>>>>   }
>>>>> 
>>>>> So In order to avoid one more config flags specific to armv8 in
>>>>> meson and makefile build infra And avoid the need for 6/6 patch.
>>>>> IMO, # Introduce optional CPU flag scheme in eal. Treat armv8 crypto
>>>>> as optional flag # Skip the eal init check for optional flag.
>>>>> 
>>>>> Do you see any issues with that approach?
>>>> 
>>>> I also thought about that approach and that was my number 1 priority.
>>>> But, I had one question came to my mind. Maybe, arm people can
>>>> confirm it. Is it 100% guaranteed that compiler never makes use of
>>>> any of crypto instructions even if there's no specific asm/intrinsic
>>>> code?  The crypto extension has aes, pmull,
>>>> sha1 and sha2. In case of rte_memcpy() for x86, for example, compiler
>>>> may optimize code using avx512f instructions even though it is
>>>> written specifically with avx2 intrinsics (__mm256_*) unless avx512f is
>> disabled.
>>>> 
>>>> If a complier expert in arm (or anyone else) confirm it is completely
>>>> **optional**, then I'd love to take that approach for sure.
>>>> 
>>>> Copied dpdk-on-arm ML.
>>>> 
>>> I do not know the answer, will have to check with the compiler team. I will get
>> back on this.
>> 
>> Any update yet?
> Currently, enabling 'crypto' flag will generate the crypto instructions only when crypto intrinsics are used. However, when 'sha3' (part of 8.2 crypto) flag is enabled, compiler can generate 3-way exclusive OR instructions beyond the intrinsics. Compiler team cannot provide a guarantee that other crypto instructions will not be used beyond the intrinsics.
> 
> The current suggestion is to use GNU indirect function [1] or similar. I am not sure on GNU indirect function portability.
> 
> [1] https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwillnewton.name%2F2013%2F07%2F02%2Fusing-gnu-indirect-functions%2F&amp;data=02%7C01%7Cyskoh%40mellanox.com%7Ce8738c4f725a4ca608ea08d6cd1cac03%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636921920373635167&amp;sdata=kuq6dbpTBfRgokrv2L%2FV4BIM0q1k%2FiL1JaMqCHUc2c0%3D&amp;reserved=0

Thanks for the update,

Then, I think the original patch to have build config is currently okay.
Will submit it again.

thanks
Yongseok

Re: [dpdk-dev] [EXT] [PATCH 5/6] build: add option for armv8 crypto extension

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Tuesday, April 30, 2019 9:04 AM
> To: yskoh@mellanox.com
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> bruce.richardson@intel.com; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>; Shahaf Shuler <shahafs@mellanox.com>;
> dev@dpdk.org; thomas@monjalon.net; Gavin Hu (Arm Technology China)
> <Gavin.Hu@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; nd <nd@arm.com>
> Subject: RE: [EXT] [PATCH 5/6] build: add option for armv8 crypto extension
> 
> > On Apr 15, 2019, at 1:13 PM, Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com> wrote:
> >
> > >>>> Subject: [EXT] [PATCH 5/6] build: add option for armv8 crypto
> > >>>> extension
> > >>>>
> > >>>> CONFIG_RTE_MACHINE="armv8a"
> > >>>> +CONFIG_RTE_ENABLE_ARMV8_CRYPTO=y
> > >>>
> > >>> This approach is not scalable. Even, it is not good for BlueField
> > >>> as you you need to maintain two images.
> > >>>
> > >>> Unlike other CPU flags, arm64's crypto cpu flag is really _optional_.
> > >>> Access to crypto instructions is always at under runtime check.
> > >>> See the following in rte_armv8_pmd.c
> > >>>
> > >>>
> > >>>    /* Check CPU for support for AES instruction set */
> > >>>    if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AES)) {
> > >>>        ARMV8_CRYPTO_LOG_ERR(
> > >>>            "AES instructions not supported by CPU");
> > >>>        return -EFAULT;
> > >>>    }
> > >>>
> > >>>    /* Check CPU for support for SHA instruction set */
> > >>>    if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA1) ||
> > >>>        !rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA2)) {
> > >>>        ARMV8_CRYPTO_LOG_ERR(
> > >>>            "SHA1/SHA2 instructions not supported by CPU");
> > >>>        return -EFAULT;
> > >>>    }
> > >>>
> > >>> So In order to avoid one more config flags specific to armv8 in
> > >>> meson and makefile build infra And avoid the need for 6/6 patch.
> > >>> IMO, # Introduce optional CPU flag scheme in eal. Treat armv8
> > >>> crypto as optional flag # Skip the eal init check for optional flag.
> > >>>
> > >>> Do you see any issues with that approach?
> > >>
> > >> I also thought about that approach and that was my number 1 priority.
> > >> But, I had one question came to my mind. Maybe, arm people can
> > >> confirm it. Is it 100% guaranteed that compiler never makes use of
> > >> any of crypto instructions even if there's no specific
> > >> asm/intrinsic code?  The crypto extension has aes, pmull,
> > >> sha1 and sha2. In case of rte_memcpy() for x86, for example,
> > >> compiler may optimize code using avx512f instructions even though
> > >> it is written specifically with avx2 intrinsics (__mm256_*) unless
> > >> avx512f is
> > disabled.
> > >>
> > >> If a complier expert in arm (or anyone else) confirm it is
> > >> completely **optional**, then I'd love to take that approach for sure.
> > >>
> > >> Copied dpdk-on-arm ML.
> > >>
> > > I do not know the answer, will have to check with the compiler team.
> > > I will get
> > back on this.
> >
> > Any update yet?
> Currently, enabling 'crypto' flag will generate the crypto instructions only when
> crypto intrinsics are used. However, when 'sha3' (part of 8.2 crypto) flag is

The default image is 8.1 spec and except octeontx2 every other SoC is 8.1 and
For octeotx2 crypto is supported. If so, Should we worry this case?

> enabled, compiler can generate 3-way exclusive OR instructions beyond the
> intrinsics.

The very same problem will be applicable for Linux kernel too for distribution binary case.
If the above statement is true about 8.2 crypto and crypto generation without
Intrinsics then we need to see how linux kernel handling that and align our solution
based on that.

> Compiler team cannot provide a guarantee that other crypto
> instructions will not be used beyond the intrinsics.
> 
> The current suggestion is to use GNU indirect function [1] or similar. I am not

Not sure how it helps? If we know the compiler is generating a specific function
With crypto instruction then we can generate _alternative_ function for the same
With hwcap?.How do we know which function compiler using compiler instructions?


> sure on GNU indirect function portability.

We are using HWCAP scheme, So we may not need the very exact GNU indirect
scheme to fix the issue.

> 
> [1] https://willnewton.name/2013/07/02/using-gnu-indirect-functions/
> 
> >
> > Thanks
> > Yongseok

Re: [dpdk-dev] [EXT] [PATCH 5/6] build: add option for armv8 crypto extension

[Yongseok Koh]

> On May 2, 2019, at 3:13 AM, Jerin Jacob Kollanukkaran <jerinj@marvell.com> wrote:
> 
>> -----Original Message-----
>> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
>> Sent: Tuesday, April 30, 2019 9:04 AM
>> To: yskoh@mellanox.com
>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
>> bruce.richardson@intel.com; Pavan Nikhilesh Bhagavatula
>> <pbhagavatula@marvell.com>; Shahaf Shuler <shahafs@mellanox.com>;
>> dev@dpdk.org; thomas@monjalon.net; Gavin Hu (Arm Technology China)
>> <Gavin.Hu@arm.com>; Honnappa Nagarahalli
>> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; nd <nd@arm.com>
>> Subject: RE: [EXT] [PATCH 5/6] build: add option for armv8 crypto extension
>> 
>>> On Apr 15, 2019, at 1:13 PM, Honnappa Nagarahalli
>>> <Honnappa.Nagarahalli@arm.com> wrote:
>>> 
>>>>>>> Subject: [EXT] [PATCH 5/6] build: add option for armv8 crypto
>>>>>>> extension
>>>>>>> 
>>>>>>> CONFIG_RTE_MACHINE="armv8a"
>>>>>>> +CONFIG_RTE_ENABLE_ARMV8_CRYPTO=y
>>>>>> 
>>>>>> This approach is not scalable. Even, it is not good for BlueField
>>>>>> as you you need to maintain two images.
>>>>>> 
>>>>>> Unlike other CPU flags, arm64's crypto cpu flag is really _optional_.
>>>>>> Access to crypto instructions is always at under runtime check.
>>>>>> See the following in rte_armv8_pmd.c
>>>>>> 
>>>>>> 
>>>>>>   /* Check CPU for support for AES instruction set */
>>>>>>   if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AES)) {
>>>>>>       ARMV8_CRYPTO_LOG_ERR(
>>>>>>           "AES instructions not supported by CPU");
>>>>>>       return -EFAULT;
>>>>>>   }
>>>>>> 
>>>>>>   /* Check CPU for support for SHA instruction set */
>>>>>>   if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA1) ||
>>>>>>       !rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA2)) {
>>>>>>       ARMV8_CRYPTO_LOG_ERR(
>>>>>>           "SHA1/SHA2 instructions not supported by CPU");
>>>>>>       return -EFAULT;
>>>>>>   }
>>>>>> 
>>>>>> So In order to avoid one more config flags specific to armv8 in
>>>>>> meson and makefile build infra And avoid the need for 6/6 patch.
>>>>>> IMO, # Introduce optional CPU flag scheme in eal. Treat armv8
>>>>>> crypto as optional flag # Skip the eal init check for optional flag.
>>>>>> 
>>>>>> Do you see any issues with that approach?
>>>>> 
>>>>> I also thought about that approach and that was my number 1 priority.
>>>>> But, I had one question came to my mind. Maybe, arm people can
>>>>> confirm it. Is it 100% guaranteed that compiler never makes use of
>>>>> any of crypto instructions even if there's no specific
>>>>> asm/intrinsic code?  The crypto extension has aes, pmull,
>>>>> sha1 and sha2. In case of rte_memcpy() for x86, for example,
>>>>> compiler may optimize code using avx512f instructions even though
>>>>> it is written specifically with avx2 intrinsics (__mm256_*) unless
>>>>> avx512f is
>>> disabled.
>>>>> 
>>>>> If a complier expert in arm (or anyone else) confirm it is
>>>>> completely **optional**, then I'd love to take that approach for sure.
>>>>> 
>>>>> Copied dpdk-on-arm ML.
>>>>> 
>>>> I do not know the answer, will have to check with the compiler team.
>>>> I will get
>>> back on this.
>>> 
>>> Any update yet?
>> Currently, enabling 'crypto' flag will generate the crypto instructions only when
>> crypto intrinsics are used. However, when 'sha3' (part of 8.2 crypto) flag is
> 
> The default image is 8.1 spec and except octeontx2 every other SoC is 8.1 and
> For octeotx2 crypto is supported. If so, Should we worry this case?

Right, it sounds to me that we can disable the option without having the new
config flag until such instructions get needed. According to gcc-8 release note
[1], currently '+crypto' implies '+aes' and '+sha2' while '+sha3' and '+sm4' are
newly introduced. Given that armv8 crypto PMD uses external binary of Marvell. I
don't see any reason to enable '+crypto'. How about simply disable it from armv8
build configs?

diff --git a/config/arm/meson.build b/config/arm/meson.build
index 7fa6ed3105..abc8cf346c 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -74,7 +74,7 @@ flags_octeontx2_extra = [
        ['RTE_USE_C11_MEM_MODEL', true]]

 machine_args_generic = [
-       ['default', ['-march=armv8-a+crc+crypto']],
+       ['default', ['-march=armv8-a+crc']],
        ['native', ['-march=native']],
        ['0xd03', ['-mcpu=cortex-a53']],
        ['0xd04', ['-mcpu=cortex-a35']],
diff --git a/mk/machine/armv8a/rte.vars.mk b/mk/machine/armv8a/rte.vars.mk
index 8252efbb7b..5e3ffc3adf 100644
--- a/mk/machine/armv8a/rte.vars.mk
+++ b/mk/machine/armv8a/rte.vars.mk
@@ -28,4 +28,4 @@
 # CPU_LDFLAGS =
 # CPU_ASFLAGS =

-MACHINE_CFLAGS += -march=armv8-a+crc+crypto
+MACHINE_CFLAGS += -march=armv8-a+crc


[1] https://gcc.gnu.org/gcc-8/changes.html

Thanks,
Yongseok

>> enabled, compiler can generate 3-way exclusive OR instructions beyond the
>> intrinsics.
> 
> The very same problem will be applicable for Linux kernel too for distribution binary case.
> If the above statement is true about 8.2 crypto and crypto generation without
> Intrinsics then we need to see how linux kernel handling that and align our solution
> based on that.
> 
>> Compiler team cannot provide a guarantee that other crypto
>> instructions will not be used beyond the intrinsics.
>> 
>> The current suggestion is to use GNU indirect function [1] or similar. I am not
> 
> Not sure how it helps? If we know the compiler is generating a specific function
> With crypto instruction then we can generate _alternative_ function for the same
> With hwcap?.How do we know which function compiler using compiler instructions?
> 
> 
>> sure on GNU indirect function portability.
> 
> We are using HWCAP scheme, So we may not need the very exact GNU indirect
> scheme to fix the issue.
> 
>> 
>> [1] https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwillnewton.name%2F2013%2F07%2F02%2Fusing-gnu-indirect-functions%2F&amp;data=02%7C01%7Cyskoh%40mellanox.com%7Cda8fb7ed03e7406ded8908d6cee6d759%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636923888189316743&amp;sdata=x5XNd5WZ3EtiprPMiFzaskvigX8K0AoXA2w%2BKiN156c%3D&amp;reserved=0
>> 
>>> 
>>> Thanks
>>> Yongseok

Re: [dpdk-dev] [EXT] [PATCH 5/6] build: add option for armv8 crypto extension

[Yongseok Koh]

> On May 2, 2019, at 4:08 PM, Yongseok Koh <yskoh@mellanox.com> wrote:
> 
>> 
>> On May 2, 2019, at 3:13 AM, Jerin Jacob Kollanukkaran <jerinj@marvell.com> wrote:
>> 
>>> -----Original Message-----
>>> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
>>> Sent: Tuesday, April 30, 2019 9:04 AM
>>> To: yskoh@mellanox.com
>>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
>>> bruce.richardson@intel.com; Pavan Nikhilesh Bhagavatula
>>> <pbhagavatula@marvell.com>; Shahaf Shuler <shahafs@mellanox.com>;
>>> dev@dpdk.org; thomas@monjalon.net; Gavin Hu (Arm Technology China)
>>> <Gavin.Hu@arm.com>; Honnappa Nagarahalli
>>> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; nd <nd@arm.com>
>>> Subject: RE: [EXT] [PATCH 5/6] build: add option for armv8 crypto extension
>>> 
>>>> On Apr 15, 2019, at 1:13 PM, Honnappa Nagarahalli
>>>> <Honnappa.Nagarahalli@arm.com> wrote:
>>>> 
>>>>>>>> Subject: [EXT] [PATCH 5/6] build: add option for armv8 crypto
>>>>>>>> extension
>>>>>>>> 
>>>>>>>> CONFIG_RTE_MACHINE="armv8a"
>>>>>>>> +CONFIG_RTE_ENABLE_ARMV8_CRYPTO=y
>>>>>>> 
>>>>>>> This approach is not scalable. Even, it is not good for BlueField
>>>>>>> as you you need to maintain two images.
>>>>>>> 
>>>>>>> Unlike other CPU flags, arm64's crypto cpu flag is really _optional_.
>>>>>>> Access to crypto instructions is always at under runtime check.
>>>>>>> See the following in rte_armv8_pmd.c
>>>>>>> 
>>>>>>> 
>>>>>>>  /* Check CPU for support for AES instruction set */
>>>>>>>  if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AES)) {
>>>>>>>      ARMV8_CRYPTO_LOG_ERR(
>>>>>>>          "AES instructions not supported by CPU");
>>>>>>>      return -EFAULT;
>>>>>>>  }
>>>>>>> 
>>>>>>>  /* Check CPU for support for SHA instruction set */
>>>>>>>  if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA1) ||
>>>>>>>      !rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA2)) {
>>>>>>>      ARMV8_CRYPTO_LOG_ERR(
>>>>>>>          "SHA1/SHA2 instructions not supported by CPU");
>>>>>>>      return -EFAULT;
>>>>>>>  }
>>>>>>> 
>>>>>>> So In order to avoid one more config flags specific to armv8 in
>>>>>>> meson and makefile build infra And avoid the need for 6/6 patch.
>>>>>>> IMO, # Introduce optional CPU flag scheme in eal. Treat armv8
>>>>>>> crypto as optional flag # Skip the eal init check for optional flag.
>>>>>>> 
>>>>>>> Do you see any issues with that approach?
>>>>>> 
>>>>>> I also thought about that approach and that was my number 1 priority.
>>>>>> But, I had one question came to my mind. Maybe, arm people can
>>>>>> confirm it. Is it 100% guaranteed that compiler never makes use of
>>>>>> any of crypto instructions even if there's no specific
>>>>>> asm/intrinsic code?  The crypto extension has aes, pmull,
>>>>>> sha1 and sha2. In case of rte_memcpy() for x86, for example,
>>>>>> compiler may optimize code using avx512f instructions even though
>>>>>> it is written specifically with avx2 intrinsics (__mm256_*) unless
>>>>>> avx512f is
>>>> disabled.
>>>>>> 
>>>>>> If a complier expert in arm (or anyone else) confirm it is
>>>>>> completely **optional**, then I'd love to take that approach for sure.
>>>>>> 
>>>>>> Copied dpdk-on-arm ML.
>>>>>> 
>>>>> I do not know the answer, will have to check with the compiler team.
>>>>> I will get
>>>> back on this.
>>>> 
>>>> Any update yet?
>>> Currently, enabling 'crypto' flag will generate the crypto instructions only when
>>> crypto intrinsics are used. However, when 'sha3' (part of 8.2 crypto) flag is
>> 
>> The default image is 8.1 spec and except octeontx2 every other SoC is 8.1 and
>> For octeotx2 crypto is supported. If so, Should we worry this case?
> 
> Right, it sounds to me that we can disable the option without having the new
> config flag until such instructions get needed. According to gcc-8 release note
> [1], currently '+crypto' implies '+aes' and '+sha2' while '+sha3' and '+sm4' are
> newly introduced. Given that armv8 crypto PMD uses external binary of Marvell. I
> don't see any reason to enable '+crypto'. How about simply disable it from armv8
> build configs?
> 
> diff --git a/config/arm/meson.build b/config/arm/meson.build
> index 7fa6ed3105..abc8cf346c 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -74,7 +74,7 @@ flags_octeontx2_extra = [
>        ['RTE_USE_C11_MEM_MODEL', true]]
> 
> machine_args_generic = [
> -       ['default', ['-march=armv8-a+crc+crypto']],
> +       ['default', ['-march=armv8-a+crc']],
>        ['native', ['-march=native']],
>        ['0xd03', ['-mcpu=cortex-a53']],
>        ['0xd04', ['-mcpu=cortex-a35']],
> diff --git a/mk/machine/armv8a/rte.vars.mk b/mk/machine/armv8a/rte.vars.mk
> index 8252efbb7b..5e3ffc3adf 100644
> --- a/mk/machine/armv8a/rte.vars.mk
> +++ b/mk/machine/armv8a/rte.vars.mk
> @@ -28,4 +28,4 @@
> # CPU_LDFLAGS =
> # CPU_ASFLAGS =
> 
> -MACHINE_CFLAGS += -march=armv8-a+crc+crypto
> +MACHINE_CFLAGS += -march=armv8-a+crc
> 
> 
> [1] https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fgcc-8%2Fchanges.html&amp;data=02%7C01%7Cyskoh%40mellanox.com%7C8a0d60c82a11498bf65608d6cf5327c3%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636924353391308162&amp;sdata=cuueiNi%2FdBfEJDKa8IFstwctBIrOkfZn0J7xojxgfvI%3D&amp;reserved=0

Just to make sure, I've run examples/ipsec-secgw on BlueField and it ran well as expected.

>>> enabled, compiler can generate 3-way exclusive OR instructions beyond the
>>> intrinsics.
>> 
>> The very same problem will be applicable for Linux kernel too for distribution binary case.
>> If the above statement is true about 8.2 crypto and crypto generation without
>> Intrinsics then we need to see how linux kernel handling that and align our solution
>> based on that.
>> 
>>> Compiler team cannot provide a guarantee that other crypto
>>> instructions will not be used beyond the intrinsics.
>>> 
>>> The current suggestion is to use GNU indirect function [1] or similar. I am not
>> 
>> Not sure how it helps? If we know the compiler is generating a specific function
>> With crypto instruction then we can generate _alternative_ function for the same
>> With hwcap?.How do we know which function compiler using compiler instructions?
>> 
>> 
>>> sure on GNU indirect function portability.
>> 
>> We are using HWCAP scheme, So we may not need the very exact GNU indirect
>> scheme to fix the issue.
>> 
>>> 
>>> [1] https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwillnewton.name%2F2013%2F07%2F02%2Fusing-gnu-indirect-functions%2F&amp;data=02%7C01%7Cyskoh%40mellanox.com%7C8a0d60c82a11498bf65608d6cf5327c3%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636924353391308162&amp;sdata=WcRHom7k1MFmHzK1LYJEaI5ruMzCvvMxlFo7Ivl%2BOh4%3D&amp;reserved=0
>>> 
>>>> 
>>>> Thanks
>>>> Yongseok

Re: [dpdk-dev] [EXT] [PATCH 5/6] build: add option for armv8 crypto extension

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Yongseok Koh <yskoh@mellanox.com>
> Sent: Friday, May 3, 2019 5:03 AM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> bruce.richardson@intel.com; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>; Shahaf Shuler <shahafs@mellanox.com>;
> dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>; Gavin Hu (Arm
> Technology China) <Gavin.Hu@arm.com>; nd <nd@arm.com>
> Subject: Re: [dpdk-dev] [EXT] [PATCH 5/6] build: add option for armv8 crypto
> extension
> 
> 
> > On May 2, 2019, at 4:08 PM, Yongseok Koh <yskoh@mellanox.com> wrote:
> >
> >>
> >> On May 2, 2019, at 3:13 AM, Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> wrote:
> >>
> >>> -----Original Message-----
> >>> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> >>> Sent: Tuesday, April 30, 2019 9:04 AM
> >>> To: yskoh@mellanox.com
> >>> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> >>> bruce.richardson@intel.com; Pavan Nikhilesh Bhagavatula
> >>> <pbhagavatula@marvell.com>; Shahaf Shuler <shahafs@mellanox.com>;
> >>> dev@dpdk.org; thomas@monjalon.net; Gavin Hu (Arm Technology China)
> >>> <Gavin.Hu@arm.com>; Honnappa Nagarahalli
> >>> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; nd
> <nd@arm.com>
> >>> Subject: RE: [EXT] [PATCH 5/6] build: add option for armv8 crypto
> >>> extension
> >>>
> >>>> On Apr 15, 2019, at 1:13 PM, Honnappa Nagarahalli
> >>>> <Honnappa.Nagarahalli@arm.com> wrote:
> >>>>
> >>>>>>>> Subject: [EXT] [PATCH 5/6] build: add option for armv8 crypto
> >>>>>>>> extension
> >>>>>>>>
> >>>>>>>> CONFIG_RTE_MACHINE="armv8a"
> >>>>>>>> +CONFIG_RTE_ENABLE_ARMV8_CRYPTO=y
> >>>>>>>
> >>>>>>> This approach is not scalable. Even, it is not good for
> >>>>>>> BlueField as you you need to maintain two images.
> >>>>>>>
> >>>>>>> Unlike other CPU flags, arm64's crypto cpu flag is really _optional_.
> >>>>>>> Access to crypto instructions is always at under runtime check.
> >>>>>>> See the following in rte_armv8_pmd.c
> >>>>>>>
> >>>>>>>
> >>>>>>>  /* Check CPU for support for AES instruction set */  if
> >>>>>>> (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AES)) {
> >>>>>>>      ARMV8_CRYPTO_LOG_ERR(
> >>>>>>>          "AES instructions not supported by CPU");
> >>>>>>>      return -EFAULT;
> >>>>>>>  }
> >>>>>>>
> >>>>>>>  /* Check CPU for support for SHA instruction set */  if
> >>>>>>> (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA1) ||
> >>>>>>>      !rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA2)) {
> >>>>>>>      ARMV8_CRYPTO_LOG_ERR(
> >>>>>>>          "SHA1/SHA2 instructions not supported by CPU");
> >>>>>>>      return -EFAULT;
> >>>>>>>  }
> >>>>>>>
> >>>>>>> So In order to avoid one more config flags specific to armv8 in
> >>>>>>> meson and makefile build infra And avoid the need for 6/6 patch.
> >>>>>>> IMO, # Introduce optional CPU flag scheme in eal. Treat armv8
> >>>>>>> crypto as optional flag # Skip the eal init check for optional flag.
> >>>>>>>
> >>>>>>> Do you see any issues with that approach?
> >>>>>>
> >>>>>> I also thought about that approach and that was my number 1 priority.
> >>>>>> But, I had one question came to my mind. Maybe, arm people can
> >>>>>> confirm it. Is it 100% guaranteed that compiler never makes use
> >>>>>> of any of crypto instructions even if there's no specific
> >>>>>> asm/intrinsic code?  The crypto extension has aes, pmull,
> >>>>>> sha1 and sha2. In case of rte_memcpy() for x86, for example,
> >>>>>> compiler may optimize code using avx512f instructions even though
> >>>>>> it is written specifically with avx2 intrinsics (__mm256_*)
> >>>>>> unless avx512f is
> >>>> disabled.
> >>>>>>
> >>>>>> If a complier expert in arm (or anyone else) confirm it is
> >>>>>> completely **optional**, then I'd love to take that approach for sure.
> >>>>>>
> >>>>>> Copied dpdk-on-arm ML.
> >>>>>>
> >>>>> I do not know the answer, will have to check with the compiler team.
> >>>>> I will get
> >>>> back on this.
> >>>>
> >>>> Any update yet?
> >>> Currently, enabling 'crypto' flag will generate the crypto
> >>> instructions only when crypto intrinsics are used. However, when
> >>> 'sha3' (part of 8.2 crypto) flag is
> >>
> >> The default image is 8.1 spec and except octeontx2 every other SoC is
> >> 8.1 and For octeotx2 crypto is supported. If so, Should we worry this case?
> >
> > Right, it sounds to me that we can disable the option without having
> > the new config flag until such instructions get needed. According to
> > gcc-8 release note [1], currently '+crypto' implies '+aes' and '+sha2'
> > while '+sha3' and '+sm4' are newly introduced. Given that armv8 crypto
> > PMD uses external binary of Marvell. I don't see any reason to enable
> > '+crypto'. How about simply disable it from armv8 
build configs?
+1
Yes. Simply disable crypto would be enough for DPDK.

Re: [dpdk-dev] [EXT] [PATCH 5/6] build: add option for armv8 crypto extension

[Honnappa Nagarahalli]

> >>> On Apr 15, 2019, at 1:13 PM, Honnappa Nagarahalli
> >>> <Honnappa.Nagarahalli@arm.com> wrote:
> >>>
> >>>>>>> Subject: [EXT] [PATCH 5/6] build: add option for armv8 crypto
> >>>>>>> extension
> >>>>>>>
> >>>>>>> CONFIG_RTE_MACHINE="armv8a"
> >>>>>>> +CONFIG_RTE_ENABLE_ARMV8_CRYPTO=y
> >>>>>>
> >>>>>> This approach is not scalable. Even, it is not good for BlueField
> >>>>>> as you you need to maintain two images.
> >>>>>>
> >>>>>> Unlike other CPU flags, arm64's crypto cpu flag is really _optional_.
> >>>>>> Access to crypto instructions is always at under runtime check.
> >>>>>> See the following in rte_armv8_pmd.c
> >>>>>>
> >>>>>>
> >>>>>>   /* Check CPU for support for AES instruction set */
> >>>>>>   if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AES)) {
> >>>>>>       ARMV8_CRYPTO_LOG_ERR(
> >>>>>>           "AES instructions not supported by CPU");
> >>>>>>       return -EFAULT;
> >>>>>>   }
> >>>>>>
> >>>>>>   /* Check CPU for support for SHA instruction set */
> >>>>>>   if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA1) ||
> >>>>>>       !rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA2)) {
> >>>>>>       ARMV8_CRYPTO_LOG_ERR(
> >>>>>>           "SHA1/SHA2 instructions not supported by CPU");
> >>>>>>       return -EFAULT;
> >>>>>>   }
> >>>>>>
> >>>>>> So In order to avoid one more config flags specific to armv8 in
> >>>>>> meson and makefile build infra And avoid the need for 6/6 patch.
> >>>>>> IMO, # Introduce optional CPU flag scheme in eal. Treat armv8
> >>>>>> crypto as optional flag # Skip the eal init check for optional flag.
> >>>>>>
> >>>>>> Do you see any issues with that approach?
> >>>>>
> >>>>> I also thought about that approach and that was my number 1 priority.
> >>>>> But, I had one question came to my mind. Maybe, arm people can
> >>>>> confirm it. Is it 100% guaranteed that compiler never makes use of
> >>>>> any of crypto instructions even if there's no specific
> >>>>> asm/intrinsic code?  The crypto extension has aes, pmull,
> >>>>> sha1 and sha2. In case of rte_memcpy() for x86, for example,
> >>>>> compiler may optimize code using avx512f instructions even though
> >>>>> it is written specifically with avx2 intrinsics (__mm256_*) unless
> >>>>> avx512f is
> >>> disabled.
> >>>>>
> >>>>> If a complier expert in arm (or anyone else) confirm it is
> >>>>> completely **optional**, then I'd love to take that approach for sure.
> >>>>>
> >>>>> Copied dpdk-on-arm ML.
> >>>>>
> >>>> I do not know the answer, will have to check with the compiler team.
> >>>> I will get
> >>> back on this.
> >>>
> >>> Any update yet?
> >> Currently, enabling 'crypto' flag will generate the crypto
> >> instructions only when crypto intrinsics are used. However, when
> >> 'sha3' (part of 8.2 crypto) flag is
> >
> > The default image is 8.1 spec and except octeontx2 every other SoC is
I am not following this. I think the default image is 8.0.

> > 8.1 and For octeotx2 crypto is supported. If so, Should we worry this case?
I assume we all are talking about the distro/binary portable build. IMO, we should not just look at the existing SoCs.
The CPU specific builds have the freedom to compile as per their corresponding support.

> 
> Right, it sounds to me that we can disable the option without having the new
> config flag until such instructions get needed. According to gcc-8 release note
> [1], currently '+crypto' implies '+aes' and '+sha2' while '+sha3' and '+sm4' are
> newly introduced. Given that armv8 crypto PMD uses external binary of
> Marvell. I don't see any reason to enable '+crypto'. How about simply disable
> it from armv8 build configs?
I think it should be fine. But, this alone is not enough. The run time detection of the crypto feature and hooking up the correct pointers needs to be added.

> 
> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> 7fa6ed3105..abc8cf346c 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -74,7 +74,7 @@ flags_octeontx2_extra = [
>         ['RTE_USE_C11_MEM_MODEL', true]]
> 
>  machine_args_generic = [
> -       ['default', ['-march=armv8-a+crc+crypto']],
> +       ['default', ['-march=armv8-a+crc']],
>         ['native', ['-march=native']],
>         ['0xd03', ['-mcpu=cortex-a53']],
>         ['0xd04', ['-mcpu=cortex-a35']], diff --git
> a/mk/machine/armv8a/rte.vars.mk b/mk/machine/armv8a/rte.vars.mk index
> 8252efbb7b..5e3ffc3adf 100644
> --- a/mk/machine/armv8a/rte.vars.mk
> +++ b/mk/machine/armv8a/rte.vars.mk
> @@ -28,4 +28,4 @@
>  # CPU_LDFLAGS =
>  # CPU_ASFLAGS =
> 
> -MACHINE_CFLAGS += -march=armv8-a+crc+crypto
> +MACHINE_CFLAGS += -march=armv8-a+crc
> 
> 
> [1] https://gcc.gnu.org/gcc-8/changes.html
> 
> Thanks,
> Yongseok
> 
> >> enabled, compiler can generate 3-way exclusive OR instructions beyond
> >> the intrinsics.
> >
> > The very same problem will be applicable for Linux kernel too for
> distribution binary case.
> > If the above statement is true about 8.2 crypto and crypto generation
> > without Intrinsics then we need to see how linux kernel handling that
> > and align our solution based on that.
Yes, the compiler team cited Linux kernel example, I have not verified it myself.

> >
> >> Compiler team cannot provide a guarantee that other crypto
> >> instructions will not be used beyond the intrinsics.
> >>
> >> The current suggestion is to use GNU indirect function [1] or
> >> similar. I am not
> >
> > Not sure how it helps? If we know the compiler is generating a
> > specific function With crypto instruction then we can generate
> > _alternative_ function for the same With hwcap?.How do we know which
> function compiler using compiler instructions?
This feature is similar to using function pointers and choosing which function pointer to use at run time. If this feature is used, the function pointer to use is decided during dynamic linking stage.
Either ways, we need to have 2 sets of crypto PMD drivers. One that implements the actual functionality using crypto intrinsics/assembly. Only, this code needs to be compiled with '+crypto'. Second driver that implements just stubs and returns error. This code will be compiled without '+crypto'. At run time, depending on the HWCAP, the correct driver/function pointers need to be hooked up.

> >
> >
> >> sure on GNU indirect function portability.
> >
> > We are using HWCAP scheme, So we may not need the very exact GNU
> > indirect scheme to fix the issue.
Agree, using indirect functions is not a must.

> >
> >>
> >> [1]
> >> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwil
> >> lnewton.name%2F2013%2F07%2F02%2Fusing-gnu-indirect-
> functions%2F&amp;d
> >>
> ata=02%7C01%7Cyskoh%40mellanox.com%7Cda8fb7ed03e7406ded8908d6c
> ee6d759
> >> %7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C63692388818
> 9316743&amp;
> >>
> sdata=x5XNd5WZ3EtiprPMiFzaskvigX8K0AoXA2w%2BKiN156c%3D&amp;res
> erved=0
> >>
> >>>
> >>> Thanks
> >>> Yongseok

Re: [dpdk-dev] [EXT] [PATCH 5/6] build: add option for armv8 crypto extension

[Yongseok Koh]

On Fri, May 03, 2019 at 03:54:09AM +0000, Honnappa Nagarahalli wrote:
> > >>> On Apr 15, 2019, at 1:13 PM, Honnappa Nagarahalli
> > >>> <Honnappa.Nagarahalli@arm.com> wrote:
> > >>>
> > >>>>>>> Subject: [EXT] [PATCH 5/6] build: add option for armv8 crypto
> > >>>>>>> extension
> > >>>>>>>
> > >>>>>>> CONFIG_RTE_MACHINE="armv8a"
> > >>>>>>> +CONFIG_RTE_ENABLE_ARMV8_CRYPTO=y
> > >>>>>>
> > >>>>>> This approach is not scalable. Even, it is not good for BlueField
> > >>>>>> as you you need to maintain two images.
> > >>>>>>
> > >>>>>> Unlike other CPU flags, arm64's crypto cpu flag is really _optional_.
> > >>>>>> Access to crypto instructions is always at under runtime check.
> > >>>>>> See the following in rte_armv8_pmd.c
> > >>>>>>
> > >>>>>>
> > >>>>>>   /* Check CPU for support for AES instruction set */
> > >>>>>>   if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AES)) {
> > >>>>>>       ARMV8_CRYPTO_LOG_ERR(
> > >>>>>>           "AES instructions not supported by CPU");
> > >>>>>>       return -EFAULT;
> > >>>>>>   }
> > >>>>>>
> > >>>>>>   /* Check CPU for support for SHA instruction set */
> > >>>>>>   if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA1) ||
> > >>>>>>       !rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA2)) {
> > >>>>>>       ARMV8_CRYPTO_LOG_ERR(
> > >>>>>>           "SHA1/SHA2 instructions not supported by CPU");
> > >>>>>>       return -EFAULT;
> > >>>>>>   }
> > >>>>>>
> > >>>>>> So In order to avoid one more config flags specific to armv8 in
> > >>>>>> meson and makefile build infra And avoid the need for 6/6 patch.
> > >>>>>> IMO, # Introduce optional CPU flag scheme in eal. Treat armv8
> > >>>>>> crypto as optional flag # Skip the eal init check for optional flag.
> > >>>>>>
> > >>>>>> Do you see any issues with that approach?
> > >>>>>
> > >>>>> I also thought about that approach and that was my number 1 priority.
> > >>>>> But, I had one question came to my mind. Maybe, arm people can
> > >>>>> confirm it. Is it 100% guaranteed that compiler never makes use of
> > >>>>> any of crypto instructions even if there's no specific
> > >>>>> asm/intrinsic code?  The crypto extension has aes, pmull,
> > >>>>> sha1 and sha2. In case of rte_memcpy() for x86, for example,
> > >>>>> compiler may optimize code using avx512f instructions even though
> > >>>>> it is written specifically with avx2 intrinsics (__mm256_*) unless
> > >>>>> avx512f is
> > >>> disabled.
> > >>>>>
> > >>>>> If a complier expert in arm (or anyone else) confirm it is
> > >>>>> completely **optional**, then I'd love to take that approach for sure.
> > >>>>>
> > >>>>> Copied dpdk-on-arm ML.
> > >>>>>
> > >>>> I do not know the answer, will have to check with the compiler team.
> > >>>> I will get
> > >>> back on this.
> > >>>
> > >>> Any update yet?
> > >> Currently, enabling 'crypto' flag will generate the crypto
> > >> instructions only when crypto intrinsics are used. However, when
> > >> 'sha3' (part of 8.2 crypto) flag is
> > >
> > > The default image is 8.1 spec and except octeontx2 every other SoC is
> I am not following this. I think the default image is 8.0.
> 
> > > 8.1 and For octeotx2 crypto is supported. If so, Should we worry this case?
> I assume we all are talking about the distro/binary portable build. IMO, we should not just look at the existing SoCs.
> The CPU specific builds have the freedom to compile as per their corresponding support.
> 
> > 
> > Right, it sounds to me that we can disable the option without having the new
> > config flag until such instructions get needed. According to gcc-8 release note
> > [1], currently '+crypto' implies '+aes' and '+sha2' while '+sha3' and '+sm4' are
> > newly introduced. Given that armv8 crypto PMD uses external binary of
> > Marvell. I don't see any reason to enable '+crypto'. How about simply disable
> > it from armv8 build configs?
> I think it should be fine. But, this alone is not enough. The run time
> detection of the crypto feature and hooking up the correct pointers needs to
> be added.

Like Jerin pointed out above, armv8 cryptodev already has runtime check of
cpuflags. If there's no support, it returns error. Unless we need a fallback
function with non-crypto instructions instead of returning error, I don't think
such hookup of func pointers are needed.

> > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > 7fa6ed3105..abc8cf346c 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -74,7 +74,7 @@ flags_octeontx2_extra = [
> >         ['RTE_USE_C11_MEM_MODEL', true]]
> > 
> >  machine_args_generic = [
> > -       ['default', ['-march=armv8-a+crc+crypto']],
> > +       ['default', ['-march=armv8-a+crc']],
> >         ['native', ['-march=native']],
> >         ['0xd03', ['-mcpu=cortex-a53']],
> >         ['0xd04', ['-mcpu=cortex-a35']], diff --git
> > a/mk/machine/armv8a/rte.vars.mk b/mk/machine/armv8a/rte.vars.mk index
> > 8252efbb7b..5e3ffc3adf 100644
> > --- a/mk/machine/armv8a/rte.vars.mk
> > +++ b/mk/machine/armv8a/rte.vars.mk
> > @@ -28,4 +28,4 @@
> >  # CPU_LDFLAGS =
> >  # CPU_ASFLAGS =
> > 
> > -MACHINE_CFLAGS += -march=armv8-a+crc+crypto
> > +MACHINE_CFLAGS += -march=armv8-a+crc
> > 
> > 
> > [1] https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fgcc-8%2Fchanges.html&amp;data=02%7C01%7Cyskoh%40mellanox.com%7C5cd398e4cf1e45c1755a08d6cf7b0091%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636924524543262594&amp;sdata=4m4S2VQUVBMLYqpxmeLoAPqAcKGm9u1Wo5R7oE2CK94%3D&amp;reserved=0
> > 
> > Thanks,
> > Yongseok
> > 
> > >> enabled, compiler can generate 3-way exclusive OR instructions beyond
> > >> the intrinsics.
> > >
> > > The very same problem will be applicable for Linux kernel too for
> > distribution binary case.
> > > If the above statement is true about 8.2 crypto and crypto generation
> > > without Intrinsics then we need to see how linux kernel handling that
> > > and align our solution based on that.
> Yes, the compiler team cited Linux kernel example, I have not verified it myself.
> 
> > >
> > >> Compiler team cannot provide a guarantee that other crypto
> > >> instructions will not be used beyond the intrinsics.
> > >>
> > >> The current suggestion is to use GNU indirect function [1] or
> > >> similar. I am not
> > >
> > > Not sure how it helps? If we know the compiler is generating a
> > > specific function With crypto instruction then we can generate
> > > _alternative_ function for the same With hwcap?.How do we know which
> > > function compiler using compiler instructions?
> This feature is similar to using function pointers and choosing which function
> pointer to use at run time. If this feature is used, the function pointer to
> use is decided during dynamic linking stage.

I think what Jerin meant was about the case where compiler can generate crypto
instructions beyond intrinsics/asm like sha3 for 3-way exclusive OR
instructions. In this case, such function pointer can't help as we can't know
how compiler generates such instructions.

> Either ways, we need to have 2 sets of crypto PMD drivers. One that implements
> the actual functionality using crypto intrinsics/assembly. Only, this code
> needs to be compiled with '+crypto'. Second driver that implements just stubs
> and returns error. This code will be compiled without '+crypto'. At run time,
> depending on the HWCAP, the correct driver/function pointers need to be hooked
> up.

Like I mentioned above, it may not be necessary. armv8 cryptodev links external
library, which is compiled separately (out of dpdk) with crypto support and we
don't have/need a fallback but returns error if no crypto support in runtime.

> > >> sure on GNU indirect function portability.
> > >
> > > We are using HWCAP scheme, So we may not need the very exact GNU
> > > indirect scheme to fix the issue.
> Agree, using indirect functions is not a must.
> 
> > >
> > >>
> > >> [1]
> > >> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwil
> > >> lnewton.name%2F2013%2F07%2F02%2Fusing-gnu-indirect-
> > functions%2F&amp;d
> > >>
> > ata=02%7C01%7Cyskoh%40mellanox.com%7Cda8fb7ed03e7406ded8908d6c
> > ee6d759
> > >> %7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C63692388818
> > 9316743&amp;
> > >>
> > sdata=x5XNd5WZ3EtiprPMiFzaskvigX8K0AoXA2w%2BKiN156c%3D&amp;res
> > erved=0

Re: [dpdk-dev] [EXT] [PATCH 5/6] build: add option for armv8 crypto extension

[Honnappa Nagarahalli]

> On Fri, May 03, 2019 at 03:54:09AM +0000, Honnappa Nagarahalli wrote:
> > > >>> On Apr 15, 2019, at 1:13 PM, Honnappa Nagarahalli
> > > >>> <Honnappa.Nagarahalli@arm.com> wrote:
> > > >>>
> > > >>>>>>> Subject: [EXT] [PATCH 5/6] build: add option for armv8
> > > >>>>>>> crypto extension
> > > >>>>>>>
> > > >>>>>>> CONFIG_RTE_MACHINE="armv8a"
> > > >>>>>>> +CONFIG_RTE_ENABLE_ARMV8_CRYPTO=y
> > > >>>>>>
> > > >>>>>> This approach is not scalable. Even, it is not good for
> > > >>>>>> BlueField as you you need to maintain two images.
> > > >>>>>>
> > > >>>>>> Unlike other CPU flags, arm64's crypto cpu flag is really
> _optional_.
> > > >>>>>> Access to crypto instructions is always at under runtime check.
> > > >>>>>> See the following in rte_armv8_pmd.c
> > > >>>>>>
> > > >>>>>>
> > > >>>>>>   /* Check CPU for support for AES instruction set */
> > > >>>>>>   if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AES)) {
> > > >>>>>>       ARMV8_CRYPTO_LOG_ERR(
> > > >>>>>>           "AES instructions not supported by CPU");
> > > >>>>>>       return -EFAULT;
> > > >>>>>>   }
> > > >>>>>>
> > > >>>>>>   /* Check CPU for support for SHA instruction set */
> > > >>>>>>   if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA1) ||
> > > >>>>>>       !rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA2)) {
> > > >>>>>>       ARMV8_CRYPTO_LOG_ERR(
> > > >>>>>>           "SHA1/SHA2 instructions not supported by CPU");
> > > >>>>>>       return -EFAULT;
> > > >>>>>>   }
> > > >>>>>>
> > > >>>>>> So In order to avoid one more config flags specific to armv8
> > > >>>>>> in meson and makefile build infra And avoid the need for 6/6
> patch.
> > > >>>>>> IMO, # Introduce optional CPU flag scheme in eal. Treat armv8
> > > >>>>>> crypto as optional flag # Skip the eal init check for optional flag.
> > > >>>>>>
> > > >>>>>> Do you see any issues with that approach?
> > > >>>>>
> > > >>>>> I also thought about that approach and that was my number 1
> priority.
> > > >>>>> But, I had one question came to my mind. Maybe, arm people can
> > > >>>>> confirm it. Is it 100% guaranteed that compiler never makes
> > > >>>>> use of any of crypto instructions even if there's no specific
> > > >>>>> asm/intrinsic code?  The crypto extension has aes, pmull,
> > > >>>>> sha1 and sha2. In case of rte_memcpy() for x86, for example,
> > > >>>>> compiler may optimize code using avx512f instructions even
> > > >>>>> though it is written specifically with avx2 intrinsics
> > > >>>>> (__mm256_*) unless avx512f is
> > > >>> disabled.
> > > >>>>>
> > > >>>>> If a complier expert in arm (or anyone else) confirm it is
> > > >>>>> completely **optional**, then I'd love to take that approach for
> sure.
> > > >>>>>
> > > >>>>> Copied dpdk-on-arm ML.
> > > >>>>>
> > > >>>> I do not know the answer, will have to check with the compiler team.
> > > >>>> I will get
> > > >>> back on this.
> > > >>>
> > > >>> Any update yet?
> > > >> Currently, enabling 'crypto' flag will generate the crypto
> > > >> instructions only when crypto intrinsics are used. However, when
> > > >> 'sha3' (part of 8.2 crypto) flag is
> > > >
> > > > The default image is 8.1 spec and except octeontx2 every other SoC
> > > > is
> > I am not following this. I think the default image is 8.0.
> >
> > > > 8.1 and For octeotx2 crypto is supported. If so, Should we worry this
> case?
> > I assume we all are talking about the distro/binary portable build. IMO, we
> should not just look at the existing SoCs.
> > The CPU specific builds have the freedom to compile as per their
> corresponding support.
> >
> > >
> > > Right, it sounds to me that we can disable the option without having
> > > the new config flag until such instructions get needed. According to
> > > gcc-8 release note [1], currently '+crypto' implies '+aes' and
> > > '+sha2' while '+sha3' and '+sm4' are newly introduced. Given that
> > > armv8 crypto PMD uses external binary of Marvell. I don't see any
> > > reason to enable '+crypto'. How about simply disable it from armv8 build
> configs?
> > I think it should be fine. But, this alone is not enough. The run time
> > detection of the crypto feature and hooking up the correct pointers
> > needs to be added.
> 
> Like Jerin pointed out above, armv8 cryptodev already has runtime check of
> cpuflags. If there's no support, it returns error. Unless we need a fallback
> function with non-crypto instructions instead of returning error, I don't think
> such hookup of func pointers are needed.
> 
> > > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > > 7fa6ed3105..abc8cf346c 100644
> > > --- a/config/arm/meson.build
> > > +++ b/config/arm/meson.build
> > > @@ -74,7 +74,7 @@ flags_octeontx2_extra = [
> > >         ['RTE_USE_C11_MEM_MODEL', true]]
> > >
> > >  machine_args_generic = [
> > > -       ['default', ['-march=armv8-a+crc+crypto']],
> > > +       ['default', ['-march=armv8-a+crc']],
> > >         ['native', ['-march=native']],
> > >         ['0xd03', ['-mcpu=cortex-a53']],
> > >         ['0xd04', ['-mcpu=cortex-a35']], diff --git
> > > a/mk/machine/armv8a/rte.vars.mk b/mk/machine/armv8a/rte.vars.mk
> > > index 8252efbb7b..5e3ffc3adf 100644
> > > --- a/mk/machine/armv8a/rte.vars.mk
> > > +++ b/mk/machine/armv8a/rte.vars.mk
> > > @@ -28,4 +28,4 @@
> > >  # CPU_LDFLAGS =
> > >  # CPU_ASFLAGS =
> > >
> > > -MACHINE_CFLAGS += -march=armv8-a+crc+crypto
> > > +MACHINE_CFLAGS += -march=armv8-a+crc
> > >
> > >
> > > [1]
> > > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgc
> > > c.gnu.org%2Fgcc-
> 8%2Fchanges.html&amp;data=02%7C01%7Cyskoh%40mellanox
> > > .com%7C5cd398e4cf1e45c1755a08d6cf7b0091%7Ca652971c7d2e4d9ba
> 6a4d14925
> > >
> 6f461b%7C0%7C0%7C636924524543262594&amp;sdata=4m4S2VQUVBML
> YqpxmeLoAP
> > > qAcKGm9u1Wo5R7oE2CK94%3D&amp;reserved=0
> > >
> > > Thanks,
> > > Yongseok
> > >
> > > >> enabled, compiler can generate 3-way exclusive OR instructions
> > > >> beyond the intrinsics.
> > > >
> > > > The very same problem will be applicable for Linux kernel too for
> > > distribution binary case.
> > > > If the above statement is true about 8.2 crypto and crypto
> > > > generation without Intrinsics then we need to see how linux kernel
> > > > handling that and align our solution based on that.
> > Yes, the compiler team cited Linux kernel example, I have not verified it
> myself.
> >
> > > >
> > > >> Compiler team cannot provide a guarantee that other crypto
> > > >> instructions will not be used beyond the intrinsics.
> > > >>
> > > >> The current suggestion is to use GNU indirect function [1] or
> > > >> similar. I am not
> > > >
> > > > Not sure how it helps? If we know the compiler is generating a
> > > > specific function With crypto instruction then we can generate
> > > > _alternative_ function for the same With hwcap?.How do we know
> > > > which function compiler using compiler instructions?
> > This feature is similar to using function pointers and choosing which
> > function pointer to use at run time. If this feature is used, the
> > function pointer to use is decided during dynamic linking stage.
> 
> I think what Jerin meant was about the case where compiler can generate
> crypto instructions beyond intrinsics/asm like sha3 for 3-way exclusive OR
> instructions. In this case, such function pointer can't help as we can't know
> how compiler generates such instructions.
> 
> > Either ways, we need to have 2 sets of crypto PMD drivers. One that
> > implements the actual functionality using crypto intrinsics/assembly.
> > Only, this code needs to be compiled with '+crypto'. Second driver
> > that implements just stubs and returns error. This code will be
> > compiled without '+crypto'. At run time, depending on the HWCAP, the
> > correct driver/function pointers need to be hooked up.
> 
> Like I mentioned above, it may not be necessary. armv8 cryptodev links
> external library, which is compiled separately (out of dpdk) with crypto
> support and we don't have/need a fallback but returns error if no crypto
> support in runtime.
Ok, got it (did not realize crypto library is external to DPDK).

> 
> > > >> sure on GNU indirect function portability.
> > > >
> > > > We are using HWCAP scheme, So we may not need the very exact GNU
> > > > indirect scheme to fix the issue.
> > Agree, using indirect functions is not a must.
> >
> > > >
> > > >>
> > > >> [1]
> > > >> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2
> > > >> Fwil
> > > >> lnewton.name%2F2013%2F07%2F02%2Fusing-gnu-indirect-
> > > functions%2F&amp;d
> > > >>
> > >
> ata=02%7C01%7Cyskoh%40mellanox.com%7Cda8fb7ed03e7406ded8908d6c
> > > ee6d759
> > > >> %7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C63692388
> 818
> > > 9316743&amp;
> > > >>
> > >
> sdata=x5XNd5WZ3EtiprPMiFzaskvigX8K0AoXA2w%2BKiN156c%3D&amp;res
> > > erved=0

[dpdk-dev] [PATCH 6/6] mk: disable armv8 crypto extension for Mellanox BlueField

[Yongseok Koh]

Mellanox BlueField has a variant which doesn't have armv8 crypto extension.
If crypto enabled binary runs on such a pltform, rte_eal_init() fails. To
have binary compatibility across multiple variants, it is disabled by
default and can be enabled for crypto enabled parts.

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 config/defconfig_arm64-bluefield-linuxapp-gcc | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/config/defconfig_arm64-bluefield-linuxapp-gcc b/config/defconfig_arm64-bluefield-linuxapp-gcc
index b496538819..6da9c2026d 100644
--- a/config/defconfig_arm64-bluefield-linuxapp-gcc
+++ b/config/defconfig_arm64-bluefield-linuxapp-gcc
@@ -10,6 +10,12 @@ CONFIG_RTE_ARCH_ARM_TUNE="cortex-a72"
 CONFIG_RTE_MAX_NUMA_NODES=1
 CONFIG_RTE_CACHE_LINE_SIZE=64
 
+# Crypto extension of armv8
+#
+# Disabled by default for binary compatibility.
+# Can be enabled for crypto-enabled parts.
+CONFIG_RTE_ENABLE_ARMV8_CRYPTO=n
+
 # UMA architecture
 CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n
 CONFIG_RTE_LIBRTE_VHOST_NUMA=n
-- 
2.21.0.196.g041f5ea

Re: [dpdk-dev] [EXT] [PATCH 0/6] build: fix build for arm64

[Jerin Jacob Kollanukkaran]

Other than 1/1, I don't think, this patches series fixing any build for arm64.
It is adding features required for Mellanox BlueField support.
Please change subject to more appropriate name.


> -----Original Message-----
> From: Yongseok Koh <yskoh@mellanox.com>
> Sent: Saturday, April 13, 2019 4:55 AM
> To: bruce.richardson@intel.com; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>; shahafs@mellanox.com
> Cc: dev@dpdk.org; thomas@monjalon.net; gavin.hu@arm.com;
> Honnappa.Nagarahalli@arm.com
> Subject: [EXT] [PATCH 0/6] build: fix build for arm64
> 
> External Email
> 
> ----------------------------------------------------------------------
> This patchset depends on
> "meson: add infra to support machine specific flags" [1]
> 
> [1] http://patches.dpdk.org/patch/52606/
> 
> Yongseok Koh (6):
>   meson: disable octeontx for buggy compilers on arm64
>   meson: change default cache line size for cortex-a72
>   net/mlx: fix library search in meson build
>   meson: add Mellanox BlueField cross-compile config
>   build: add option for armv8 crypto extension
>   mk: disable armv8 crypto extension for Mellanox BlueField
> 
>  config/arm/arm64_bluefield_linux_gcc          | 16 ++++++++++++++++
>  config/arm/meson.build                        | 18 +++++++++++-------
>  config/common_armv8a_linux                    |  1 +
>  config/defconfig_arm64-bluefield-linuxapp-gcc |  6 ++++++
>  drivers/crypto/armv8/Makefile                 |  4 ++++
>  drivers/event/meson.build                     |  6 +++++-
>  drivers/net/mlx4/meson.build                  | 19 +++++++++++--------
>  drivers/net/mlx5/meson.build                  | 19 +++++++++++--------
>  meson_options.txt                             |  2 ++
>  mk/machine/armv8a/rte.vars.mk                 |  4 ++++
>  10 files changed, 71 insertions(+), 24 deletions(-)  create mode 100644
> config/arm/arm64_bluefield_linux_gcc
> 
> --
> 2.21.0.196.g041f5ea

Re: [dpdk-dev] [EXT] [PATCH 0/6] build: fix build for arm64

[Yongseok Koh]

> On Apr 13, 2019, at 12:12 AM, Jerin Jacob Kollanukkaran <jerinj@marvell.com> wrote:
> 
> Other than 1/1, I don't think, this patches series fixing any build for arm64.

That's one of reasons for the title.

> It is adding features required for Mellanox BlueField support.

Hard to agree.

> Please change subject to more appropriate name.

If the title of the cover letter (which isn't merged anyway but informative)
still bothers you, let me know. I'd rather remove the cover letter like your
patchset.

>> -----Original Message-----
>> From: Yongseok Koh <yskoh@mellanox.com>
>> Sent: Saturday, April 13, 2019 4:55 AM
>> To: bruce.richardson@intel.com; Jerin Jacob Kollanukkaran
>> <jerinj@marvell.com>; Pavan Nikhilesh Bhagavatula
>> <pbhagavatula@marvell.com>; shahafs@mellanox.com
>> Cc: dev@dpdk.org; thomas@monjalon.net; gavin.hu@arm.com;
>> Honnappa.Nagarahalli@arm.com
>> Subject: [EXT] [PATCH 0/6] build: fix build for arm64
>> 
>> External Email
>> 
>> ----------------------------------------------------------------------
>> This patchset depends on
>> "meson: add infra to support machine specific flags" [1]
>> 
>> [1] https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatches.dpdk.org%2Fpatch%2F52606%2F&amp;data=02%7C01%7Cyskoh%40mellanox.com%7C0c76f968187240046bfd08d6bfdf7283%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636907363757515638&amp;sdata=X3fQ%2B%2B7RIs8MbTy%2Bok5WVm6wSmslrcyRuIuECAVhxXA%3D&amp;reserved=0
>> 
>> Yongseok Koh (6):
>>  meson: disable octeontx for buggy compilers on arm64
>>  meson: change default cache line size for cortex-a72
>>  net/mlx: fix library search in meson build
>>  meson: add Mellanox BlueField cross-compile config
>>  build: add option for armv8 crypto extension
>>  mk: disable armv8 crypto extension for Mellanox BlueField
>> 
>> config/arm/arm64_bluefield_linux_gcc          | 16 ++++++++++++++++
>> config/arm/meson.build                        | 18 +++++++++++-------
>> config/common_armv8a_linux                    |  1 +
>> config/defconfig_arm64-bluefield-linuxapp-gcc |  6 ++++++
>> drivers/crypto/armv8/Makefile                 |  4 ++++
>> drivers/event/meson.build                     |  6 +++++-
>> drivers/net/mlx4/meson.build                  | 19 +++++++++++--------
>> drivers/net/mlx5/meson.build                  | 19 +++++++++++--------
>> meson_options.txt                             |  2 ++
>> mk/machine/armv8a/rte.vars.mk                 |  4 ++++
>> 10 files changed, 71 insertions(+), 24 deletions(-)  create mode 100644
>> config/arm/arm64_bluefield_linux_gcc
>> 
>> --
>> 2.21.0.196.g041f5ea
> 

Re: [dpdk-dev] [EXT] [PATCH 0/6] build: fix build for arm64

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Yongseok Koh <yskoh@mellanox.com>
> Sent: Tuesday, April 16, 2019 2:26 AM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Cc: bruce.richardson@intel.com; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>; Shahaf Shuler <shahafs@mellanox.com>;
> dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>;
> gavin.hu@arm.com; Honnappa.Nagarahalli@arm.com
> Subject: Re: [EXT] [PATCH 0/6] build: fix build for arm64
> 
> 
> > On Apr 13, 2019, at 12:12 AM, Jerin Jacob Kollanukkaran
> <jerinj@marvell.com> wrote:
> >
> > Other than 1/1, I don't think, this patches series fixing any build for arm64.
> 
> That's one of reasons for the title.
> 
> > It is adding features required for Mellanox BlueField support.
> 
> Hard to agree.

The data should say:
meson: disable octeontx for buggy compilers on arm64
meson: change default cache line size for cortex-a72
net/mlx: fix library search in meson build
meson: add Mellanox BlueField cross-compile config
build: add option for armv8 crypto extension
mk: disable armv8 crypto extension for Mellanox BlueField

> 
> > Please change subject to more appropriate name.
> 
> If the title of the cover letter (which isn't merged anyway but informative) still
> bothers you, let me know. I'd rather remove the cover letter like your patchset.

If those are fixes then it needs to back ported to stable tree.
What bothers to me as maintainer that it painting a different picture that
arm64 still has build issues for meson. It is not the case.
So it its good to say what the actual content is, Let me ask you way around,
If it is just informational not going to merged then why to change to something
more appropriate.

I don't have any strong opinion keep the cover letter or not? You can decide.




> 
> >> -----Original Message-----
> >> From: Yongseok Koh <yskoh@mellanox.com>
> >> Sent: Saturday, April 13, 2019 4:55 AM
> >> To: bruce.richardson@intel.com; Jerin Jacob Kollanukkaran
> >> <jerinj@marvell.com>; Pavan Nikhilesh Bhagavatula
> >> <pbhagavatula@marvell.com>; shahafs@mellanox.com
> >> Cc: dev@dpdk.org; thomas@monjalon.net; gavin.hu@arm.com;
> >> Honnappa.Nagarahalli@arm.com
> >> Subject: [EXT] [PATCH 0/6] build: fix build for arm64
> >>
> >> External Email
> >>
> >> ---------------------------------------------------------------------
> >> -
> >> This patchset depends on
> >> "meson: add infra to support machine specific flags" [1]
> >>
> >> [1]
> >> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatc
> >>
> hes.dpdk.org%2Fpatch%2F52606%2F&amp;data=02%7C01%7Cyskoh%40mellan
> ox.c
> >>
> om%7C0c76f968187240046bfd08d6bfdf7283%7Ca652971c7d2e4d9ba6a4d1492
> 56f4
> >>
> 61b%7C0%7C0%7C636907363757515638&amp;sdata=X3fQ%2B%2B7RIs8MbTy
> %2Bok5W
> >> Vm6wSmslrcyRuIuECAVhxXA%3D&amp;reserved=0
> >>
> >> Yongseok Koh (6):
> >>  meson: disable octeontx for buggy compilers on arm64
> >>  meson: change default cache line size for cortex-a72
> >>  net/mlx: fix library search in meson build
> >>  meson: add Mellanox BlueField cross-compile config
> >>  build: add option for armv8 crypto extension
> >>  mk: disable armv8 crypto extension for Mellanox BlueField
> >>
> >> config/arm/arm64_bluefield_linux_gcc          | 16 ++++++++++++++++
> >> config/arm/meson.build                        | 18 +++++++++++-------
> >> config/common_armv8a_linux                    |  1 +
> >> config/defconfig_arm64-bluefield-linuxapp-gcc |  6 ++++++
> >> drivers/crypto/armv8/Makefile                 |  4 ++++
> >> drivers/event/meson.build                     |  6 +++++-
> >> drivers/net/mlx4/meson.build                  | 19 +++++++++++--------
> >> drivers/net/mlx5/meson.build                  | 19 +++++++++++--------
> >> meson_options.txt                             |  2 ++
> >> mk/machine/armv8a/rte.vars.mk                 |  4 ++++
> >> 10 files changed, 71 insertions(+), 24 deletions(-)  create mode
> >> 100644 config/arm/arm64_bluefield_linux_gcc
> >>
> >> --
> >> 2.21.0.196.g041f5ea
> >

Re: [dpdk-dev] [PATCH 0/6] build: fix build for arm64

[Thomas Monjalon]

13/04/2019 01:24, Yongseok Koh:
> This patchset depends on
> "meson: add infra to support machine specific flags" [1]
> 
> [1] http://patches.dpdk.org/patch/52606/
> 
> Yongseok Koh (6):
>   meson: disable octeontx for buggy compilers on arm64
>   meson: change default cache line size for cortex-a72
>   net/mlx: fix library search in meson build
>   meson: add Mellanox BlueField cross-compile config
>   build: add option for armv8 crypto extension
>   mk: disable armv8 crypto extension for Mellanox BlueField

There are some discussions about the crypto config (last 2 patches).
I think we should proceed with patches 1 to 4 in a v2,
so we can progress and discuss about the crypto option later.

Re: [dpdk-dev] [PATCH 0/6] build: fix build for arm64

[Honnappa Nagarahalli]

> 
> 13/04/2019 01:24, Yongseok Koh:
> > This patchset depends on
> > "meson: add infra to support machine specific flags" [1]
> >
> > [1] http://patches.dpdk.org/patch/52606/
> >
> > Yongseok Koh (6):
> >   meson: disable octeontx for buggy compilers on arm64
> >   meson: change default cache line size for cortex-a72
> >   net/mlx: fix library search in meson build
> >   meson: add Mellanox BlueField cross-compile config
> >   build: add option for armv8 crypto extension
> >   mk: disable armv8 crypto extension for Mellanox BlueField
> 
> There are some discussions about the crypto config (last 2 patches).
> I think we should proceed with patches 1 to 4 in a v2, so we can progress and
> discuss about the crypto option later.
Many compiler folks are on holiday (due to extended holiday weekend 4/19, 4/22), I do not have an answer on the crypto question yet

> 

Re: [dpdk-dev] [PATCH 0/6] build: fix build for arm64

[Yongseok Koh]

> On Apr 17, 2019, at 1:06 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 13/04/2019 01:24, Yongseok Koh:
>> This patchset depends on
>> "meson: add infra to support machine specific flags" [1]
>> 
>> [1] https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatches.dpdk.org%2Fpatch%2F52606%2F&amp;data=02%7C01%7Cyskoh%40mellanox.com%7C9827d8bf7195452905f908d6c37030f4%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636911283960800945&amp;sdata=bwiJRKspk%2FACPpJlqtLRamW7ybjkjIafbkMST6pTC9A%3D&amp;reserved=0
>> 
>> Yongseok Koh (6):
>>  meson: disable octeontx for buggy compilers on arm64
>>  meson: change default cache line size for cortex-a72
>>  net/mlx: fix library search in meson build
>>  meson: add Mellanox BlueField cross-compile config
>>  build: add option for armv8 crypto extension
>>  mk: disable armv8 crypto extension for Mellanox BlueField
> 
> There are some discussions about the crypto config (last 2 patches).
> I think we should proceed with patches 1 to 4 in a v2,
> so we can progress and discuss about the crypto option later.

Will do v2 with 4 patches.

Yongseok

[dpdk-dev] [PATCH v2 1/4] meson: disable octeontx for buggy compilers on arm64

[Yongseok Koh]

Disable octeontx for gcc 4.8.5 as compiler is emitting "internal compiler
error" for aarch64

Fixes: bd77f2d64c44 ("event/octeontx: build with meson")
Fixes: 4f760550a093 ("mk: disable OcteonTx for buggy compilers")
Fixes: f3af3e44a444 ("mk: disable OcteonTx for buggy compilers only on arm64")
Cc: pbhagavatula@marvell.com
Cc: jerinj@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---

v2:
* fix bug - enable octeontx unless the buggy compiler is used

 drivers/event/meson.build | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/event/meson.build b/drivers/event/meson.build
index 836ecbb74b..fb723f727b 100644
--- a/drivers/event/meson.build
+++ b/drivers/event/meson.build
@@ -1,7 +1,11 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation
 
-drivers = ['dpaa', 'dpaa2', 'octeontx', 'opdl', 'skeleton', 'sw', 'dsw']
+drivers = ['dpaa', 'dpaa2', 'opdl', 'skeleton', 'sw', 'dsw']
+if not (toolchain == 'gcc' and cc.version().version_compare('<4.8.6') and
+	 dpdk_conf.has('RTE_ARCH_ARM64'))
+	drivers += 'octeontx'
+endif
 std_deps = ['eventdev', 'kvargs']
 config_flag_fmt = 'RTE_LIBRTE_@0@_EVENTDEV_PMD'
 driver_name_fmt = 'rte_pmd_@0@_event'
-- 
2.11.0

[dpdk-dev] [PATCH v2 2/4] meson: change default cache line size for armv8

[Yongseok Koh]

Currently, the cache line size of armv8 CPUs having Implementor ID of 0x41
is 64 bytes.

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---

v2:
* introduce flags_arm replacing flags_generic instead of using the extra flags

 config/arm/meson.build | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/config/arm/meson.build b/config/arm/meson.build
index 22a062bad9..1db4ad2ee7 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -32,6 +32,11 @@ flags_generic = [
 	['RTE_MAX_LCORE', 256],
 	['RTE_USE_C11_MEM_MODEL', true],
 	['RTE_CACHE_LINE_SIZE', 128]]
+flags_arm = [
+	['RTE_MACHINE', '"armv8a"'],
+	['RTE_MAX_LCORE', 256],
+	['RTE_USE_C11_MEM_MODEL', true],
+	['RTE_CACHE_LINE_SIZE', 64]]
 flags_cavium = [
 	['RTE_CACHE_LINE_SIZE', 128],
 	['RTE_MAX_NUMA_NODES', 2],
@@ -88,7 +93,7 @@ machine_args_cavium = [
 
 ## Arm implementer ID (ARM DDI 0487C.a, Section G7.2.106, Page G7-5321)
 impl_generic = ['Generic armv8', flags_generic, machine_args_generic]
-impl_0x41 = ['Arm', flags_generic, machine_args_generic]
+impl_0x41 = ['Arm', flags_arm, machine_args_generic]
 impl_0x42 = ['Broadcom', flags_generic, machine_args_generic]
 impl_0x43 = ['Cavium', flags_cavium, machine_args_cavium]
 impl_0x44 = ['DEC', flags_generic, machine_args_generic]
-- 
2.11.0

Re: [dpdk-dev] [PATCH v2 2/4] meson: change default cache line size for armv8

[Honnappa Nagarahalli]

> 
> Currently, the cache line size of armv8 CPUs having Implementor ID of 0x41 is
> 64 bytes.
I guess you meant to say 128 bytes

> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
> 
> v2:
> * introduce flags_arm replacing flags_generic instead of using the extra flags
> 
>  config/arm/meson.build | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> 22a062bad9..1db4ad2ee7 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -32,6 +32,11 @@ flags_generic = [
>  	['RTE_MAX_LCORE', 256],
>  	['RTE_USE_C11_MEM_MODEL', true],
>  	['RTE_CACHE_LINE_SIZE', 128]]
> +flags_arm = [
> +	['RTE_MACHINE', '"armv8a"'],
> +	['RTE_MAX_LCORE', 256],
I am not aware of any implementations with implementor ID 0x41. Bluefield is the first one I am aware of. May be we can keep this smaller, 16?

> +	['RTE_USE_C11_MEM_MODEL', true],
> +	['RTE_CACHE_LINE_SIZE', 64]]
>  flags_cavium = [
>  	['RTE_CACHE_LINE_SIZE', 128],
>  	['RTE_MAX_NUMA_NODES', 2],
> @@ -88,7 +93,7 @@ machine_args_cavium = [
> 
>  ## Arm implementer ID (ARM DDI 0487C.a, Section G7.2.106, Page G7-5321)
> impl_generic = ['Generic armv8', flags_generic, machine_args_generic]
> -impl_0x41 = ['Arm', flags_generic, machine_args_generic]
> +impl_0x41 = ['Arm', flags_arm, machine_args_generic]
>  impl_0x42 = ['Broadcom', flags_generic, machine_args_generic]
>  impl_0x43 = ['Cavium', flags_cavium, machine_args_cavium]
>  impl_0x44 = ['DEC', flags_generic, machine_args_generic]
> --
> 2.11.0

Re: [dpdk-dev] [EXT] Re: [PATCH v2 2/4] meson: change default cache line size for armv8

[Hemant Agrawal]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Honnappa Nagarahalli
> Sent: Thursday, April 18, 2019 10:31 AM
> To: yskoh@mellanox.com; bruce.richardson@intel.com; jerinj@marvell.com;
> pbhagavatula@marvell.com; shahafs@mellanox.com
> Cc: dev@dpdk.org; thomas@monjalon.net; Gavin Hu (Arm Technology
> China) <Gavin.Hu@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; nd <nd@arm.com>
> Subject: [EXT] Re: [dpdk-dev] [PATCH v2 2/4] meson: change default cache
> line size for armv8
> 
 
> >
> > Currently, the cache line size of armv8 CPUs having Implementor ID of
> > 0x41 is
> > 64 bytes.
> I guess you meant to say 128 bytes


"the current default is 128, changing it to 64."
 
> 
> >
> > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > ---
> >
> > v2:
> > * introduce flags_arm replacing flags_generic instead of using the
> > extra flags
> >
> >  config/arm/meson.build | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > 22a062bad9..1db4ad2ee7 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -32,6 +32,11 @@ flags_generic = [
> >       ['RTE_MAX_LCORE', 256],
> >       ['RTE_USE_C11_MEM_MODEL', true],
> >       ['RTE_CACHE_LINE_SIZE', 128]]
> > +flags_arm = [
> > +     ['RTE_MACHINE', '"armv8a"'],
> > +     ['RTE_MAX_LCORE', 256],
> I am not aware of any implementations with implementor ID 0x41. Bluefield
> is the first one I am aware of. May be we can keep this smaller, 16?

NXP also support implementer as 0x41, 16 will be good. 

> 
> > +     ['RTE_USE_C11_MEM_MODEL', true],
> > +     ['RTE_CACHE_LINE_SIZE', 64]]
> >  flags_cavium = [
> >       ['RTE_CACHE_LINE_SIZE', 128],
> >       ['RTE_MAX_NUMA_NODES', 2],
> > @@ -88,7 +93,7 @@ machine_args_cavium = [
> >
> >  ## Arm implementer ID (ARM DDI 0487C.a, Section G7.2.106, Page
> > G7-5321) impl_generic = ['Generic armv8', flags_generic,
> > machine_args_generic]
> > -impl_0x41 = ['Arm', flags_generic, machine_args_generic]
> > +impl_0x41 = ['Arm', flags_arm, machine_args_generic]
> >  impl_0x42 = ['Broadcom', flags_generic, machine_args_generic]
> >  impl_0x43 = ['Cavium', flags_cavium, machine_args_cavium]
> >  impl_0x44 = ['DEC', flags_generic, machine_args_generic]
> > --
> > 2.11.0

Re: [dpdk-dev] [EXT] Re: [PATCH v2 2/4] meson: change default cache line size for armv8

[Yongseok Koh]

> On Apr 18, 2019, at 1:23 AM, Hemant Agrawal <hemant.agrawal@nxp.com> wrote:
> 
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Honnappa Nagarahalli
>> Sent: Thursday, April 18, 2019 10:31 AM
>> To: yskoh@mellanox.com; bruce.richardson@intel.com; jerinj@marvell.com;
>> pbhagavatula@marvell.com; shahafs@mellanox.com
>> Cc: dev@dpdk.org; thomas@monjalon.net; Gavin Hu (Arm Technology
>> China) <Gavin.Hu@arm.com>; Honnappa Nagarahalli
>> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; nd <nd@arm.com>
>> Subject: [EXT] Re: [dpdk-dev] [PATCH v2 2/4] meson: change default cache
>> line size for armv8
>> 
> 
>>> 
>>> Currently, the cache line size of armv8 CPUs having Implementor ID of
>>> 0x41 is
>>> 64 bytes.
>> I guess you meant to say 128 bytes
> 
> 
> "the current default is 128, changing it to 64."

Yep, the message was wrong. Will fix it.

>>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>>> ---
>>> 
>>> v2:
>>> * introduce flags_arm replacing flags_generic instead of using the
>>> extra flags
>>> 
>>> config/arm/meson.build | 7 ++++++-
>>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/config/arm/meson.build b/config/arm/meson.build index
>>> 22a062bad9..1db4ad2ee7 100644
>>> --- a/config/arm/meson.build
>>> +++ b/config/arm/meson.build
>>> @@ -32,6 +32,11 @@ flags_generic = [
>>>      ['RTE_MAX_LCORE', 256],
>>>      ['RTE_USE_C11_MEM_MODEL', true],
>>>      ['RTE_CACHE_LINE_SIZE', 128]]
>>> +flags_arm = [
>>> +     ['RTE_MACHINE', '"armv8a"'],
>>> +     ['RTE_MAX_LCORE', 256],
>> I am not aware of any implementations with implementor ID 0x41. Bluefield
>> is the first one I am aware of. May be we can keep this smaller, 16?
> 
> NXP also support implementer as 0x41, 16 will be good. 

BlueField has 16 cores so yes, it is good.

Thanks,
Yongseok

> 
>> 
>>> +     ['RTE_USE_C11_MEM_MODEL', true],
>>> +     ['RTE_CACHE_LINE_SIZE', 64]]
>>> flags_cavium = [
>>>      ['RTE_CACHE_LINE_SIZE', 128],
>>>      ['RTE_MAX_NUMA_NODES', 2],
>>> @@ -88,7 +93,7 @@ machine_args_cavium = [
>>> 
>>> ## Arm implementer ID (ARM DDI 0487C.a, Section G7.2.106, Page
>>> G7-5321) impl_generic = ['Generic armv8', flags_generic,
>>> machine_args_generic]
>>> -impl_0x41 = ['Arm', flags_generic, machine_args_generic]
>>> +impl_0x41 = ['Arm', flags_arm, machine_args_generic]
>>> impl_0x42 = ['Broadcom', flags_generic, machine_args_generic]
>>> impl_0x43 = ['Cavium', flags_cavium, machine_args_cavium]
>>> impl_0x44 = ['DEC', flags_generic, machine_args_generic]
>>> --
>>> 2.11.0

[dpdk-dev] [PATCH v2 3/4] net/mlx: fix library search in meson build

[Yongseok Koh]

If MLNX_OFED is installed, there's no .pc file installed for libraries and
dependency() can't find libraries by pkg-config. By adding fallback of
using cc.find_library(), libraries are properly located.

Fixes: e30b4e566f47 ("build: improve dependency handling")
Cc: bluca@debian.org
Cc: stable@dpdk.org

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---

v2:
* change variable names to minimize code change

 drivers/net/mlx4/meson.build | 15 +++++++++------
 drivers/net/mlx5/meson.build | 15 +++++++++------
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/net/mlx4/meson.build b/drivers/net/mlx4/meson.build
index de020701d1..9d04dd930d 100644
--- a/drivers/net/mlx4/meson.build
+++ b/drivers/net/mlx4/meson.build
@@ -13,14 +13,17 @@ if pmd_dlopen
 		'-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
 	]
 endif
-libs = [
-	dependency('libmnl', required:false),
-	dependency('libmlx4', required:false),
-	dependency('libibverbs', required:false),
-]
+libnames = [ 'libmnl', 'libmlx4', 'libibverbs' ]
+libs = []
 build = true
-foreach lib:libs
+foreach libname:libnames
+	lib = dependency(libname, required:false)
 	if not lib.found()
+		lib = cc.find_library(libname, required:false)
+	endif
+	if lib.found()
+		libs += [ lib ]
+	else
 		build = false
 	endif
 endforeach
diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
index a4c684e1b5..ee8399af27 100644
--- a/drivers/net/mlx5/meson.build
+++ b/drivers/net/mlx5/meson.build
@@ -13,14 +13,17 @@ if pmd_dlopen
 		'-DMLX5_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
 	]
 endif
-libs = [
-	dependency('libmnl', required:false),
-	dependency('libmlx5', required:false),
-	dependency('libibverbs', required:false),
-]
+libnames = [ 'libmnl', 'libmlx5', 'libibverbs' ]
+libs = []
 build = true
-foreach lib:libs
+foreach libname:libnames
+	lib = dependency(libname, required:false)
 	if not lib.found()
+		lib = cc.find_library(libname, required:false)
+	endif
+	if lib.found()
+		libs += [ lib ]
+	else
 		build = false
 	endif
 endforeach
-- 
2.11.0

[dpdk-dev] [PATCH v2 4/4] meson: add Mellanox BlueField cross-compile config

[Yongseok Koh]

Mellanox BlueField is armv8 CPU having cortex-a72. The implementor ID is
0x41 (arm) and the primary part number is 0xd08 (cortex-a72).

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
---
 config/arm/arm64_bluefield_linux_gcc | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 config/arm/arm64_bluefield_linux_gcc

diff --git a/config/arm/arm64_bluefield_linux_gcc b/config/arm/arm64_bluefield_linux_gcc
new file mode 100644
index 0000000000..304c4073d5
--- /dev/null
+++ b/config/arm/arm64_bluefield_linux_gcc
@@ -0,0 +1,16 @@
+[binaries]
+c = 'aarch64-linux-gnu-gcc'
+cpp = 'aarch64-linux-gnu-cpp'
+ar = 'aarch64-linux-gnu-gcc-ar'
+strip = 'aarch64-linux-gnu-strip'
+pcap-config = ''
+
+[host_machine]
+system = 'linux'
+cpu_family = 'aarch64'
+cpu = 'armv8-a'
+endian = 'little'
+
+[properties]
+implementor_id = '0x41'
+implementor_pn = '0xd08'
-- 
2.11.0

Re: [dpdk-dev] [EXT] [PATCH v2 1/4] meson: disable octeontx for buggy compilers on arm64

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Yongseok Koh <yskoh@mellanox.com>
> Sent: Thursday, April 18, 2019 7:17 AM
> To: bruce.richardson@intel.com; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>; shahafs@mellanox.com
> Cc: dev@dpdk.org; thomas@monjalon.net; gavin.hu@arm.com;
> Honnappa.Nagarahalli@arm.com; stable@dpdk.org
> Subject: [EXT] [PATCH v2 1/4] meson: disable octeontx for buggy compilers on
> arm64
> ----------------------------------------------------------------------
> Disable octeontx for gcc 4.8.5 as compiler is emitting "internal compiler error"
> for aarch64
> 
> Fixes: bd77f2d64c44 ("event/octeontx: build with meson")
> Fixes: 4f760550a093 ("mk: disable OcteonTx for buggy compilers")
> Fixes: f3af3e44a444 ("mk: disable OcteonTx for buggy compilers only on
> arm64")
> Cc: pbhagavatula@marvell.com
> Cc: jerinj@marvell.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>

Nit:
[master] [dpdk.org] $ ./devtools/check-git-log.sh
Wrong headline prefix:
        meson: disable octeontx for buggy compilers on arm64

With above fix:
Acked-by: Jerin Jacob <jerinj@marvell.com>		

Re: [dpdk-dev] [EXT] [PATCH v2 1/4] meson: disable octeontx for buggy compilers on arm64

[Yongseok Koh]

> On Apr 18, 2019, at 12:21 AM, Jerin Jacob Kollanukkaran <jerinj@marvell.com> wrote:
> 
> 
>> -----Original Message-----
>> From: Yongseok Koh <yskoh@mellanox.com>
>> Sent: Thursday, April 18, 2019 7:17 AM
>> To: bruce.richardson@intel.com; Jerin Jacob Kollanukkaran
>> <jerinj@marvell.com>; Pavan Nikhilesh Bhagavatula
>> <pbhagavatula@marvell.com>; shahafs@mellanox.com
>> Cc: dev@dpdk.org; thomas@monjalon.net; gavin.hu@arm.com;
>> Honnappa.Nagarahalli@arm.com; stable@dpdk.org
>> Subject: [EXT] [PATCH v2 1/4] meson: disable octeontx for buggy compilers on
>> arm64
>> ----------------------------------------------------------------------
>> Disable octeontx for gcc 4.8.5 as compiler is emitting "internal compiler error"
>> for aarch64
>> 
>> Fixes: bd77f2d64c44 ("event/octeontx: build with meson")
>> Fixes: 4f760550a093 ("mk: disable OcteonTx for buggy compilers")
>> Fixes: f3af3e44a444 ("mk: disable OcteonTx for buggy compilers only on
>> arm64")
>> Cc: pbhagavatula@marvell.com
>> Cc: jerinj@marvell.com
>> Cc: stable@dpdk.org
>> 
>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> 
> Nit:
> [master] [dpdk.org] $ ./devtools/check-git-log.sh
> Wrong headline prefix:
>        meson: disable octeontx for buggy compilers on arm64

I was aware but I thought that should be accepted. That seems to be drawback of
the script. The only way to make it silent is :
	"event/meson.build: disable octeontx for ..."
I don't think you want this, do you?

I'll keep it as is but let me know if you have better way to fix it.

thanks,
Yongseok

> 
> With above fix:
> Acked-by: Jerin Jacob <jerinj@marvell.com>		

Re: [dpdk-dev] [EXT] [PATCH v2 1/4] meson: disable octeontx for buggy compilers on arm64

[Thomas Monjalon]

18/04/2019 12:41, Yongseok Koh:
> > On Apr 18, 2019, at 12:21 AM, Jerin Jacob Kollanukkaran <jerinj@marvell.com> wrote:
> > From: Yongseok Koh <yskoh@mellanox.com>
> > Nit:
> > [master] [dpdk.org] $ ./devtools/check-git-log.sh
> > Wrong headline prefix:
> >        meson: disable octeontx for buggy compilers on arm64
> 
> I was aware but I thought that should be accepted. That seems to be drawback of
> the script. The only way to make it silent is :
> 	"event/meson.build: disable octeontx for ..."
> I don't think you want this, do you?
> 
> I'll keep it as is but let me know if you have better way to fix it.

drivers/event is the right prefix here.

I can fix it on apply.

Re: [dpdk-dev] [EXT] [PATCH v2 1/4] meson: disable octeontx for buggy compilers on arm64

[Yongseok Koh]

> On Apr 18, 2019, at 4:04 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 18/04/2019 12:41, Yongseok Koh:
>>> On Apr 18, 2019, at 12:21 AM, Jerin Jacob Kollanukkaran <jerinj@marvell.com> wrote:
>>> From: Yongseok Koh <yskoh@mellanox.com>
>>> Nit:
>>> [master] [dpdk.org] $ ./devtools/check-git-log.sh
>>> Wrong headline prefix:
>>>       meson: disable octeontx for buggy compilers on arm64
>> 
>> I was aware but I thought that should be accepted. That seems to be drawback of
>> the script. The only way to make it silent is :
>> 	"event/meson.build: disable octeontx for ..."
>> I don't think you want this, do you?
>> 
>> I'll keep it as is but let me know if you have better way to fix it.
> 
> drivers/event is the right prefix here.

I've tested that already but that failed the script either.
I guess you're saying that it should be 'drivers/event:' regardless of the prefix error?

> I can fix it on apply.

[dpdk-dev] [PATCH v3 1/4] drivers/event: disable octeontx for buggy compilers on arm64

[Yongseok Koh]

Disable octeontx for gcc 4.8.5 as compiler is emitting "internal compiler
error" for aarch64

Fixes: bd77f2d64c44 ("event/octeontx: build with meson")
Fixes: 4f760550a093 ("mk: disable OcteonTx for buggy compilers")
Fixes: f3af3e44a444 ("mk: disable OcteonTx for buggy compilers only on arm64")
Cc: pbhagavatula@marvell.com
Cc: jerinj@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
---

v3:
* fix prefix of the title

v2:
* fix bug - enable octeontx unless the buggy compiler is used

 drivers/event/meson.build | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/event/meson.build b/drivers/event/meson.build
index 836ecbb74b..fb723f727b 100644
--- a/drivers/event/meson.build
+++ b/drivers/event/meson.build
@@ -1,7 +1,11 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation
 
-drivers = ['dpaa', 'dpaa2', 'octeontx', 'opdl', 'skeleton', 'sw', 'dsw']
+drivers = ['dpaa', 'dpaa2', 'opdl', 'skeleton', 'sw', 'dsw']
+if not (toolchain == 'gcc' and cc.version().version_compare('<4.8.6') and
+	dpdk_conf.has('RTE_ARCH_ARM64'))
+	drivers += 'octeontx'
+endif
 std_deps = ['eventdev', 'kvargs']
 config_flag_fmt = 'RTE_LIBRTE_@0@_EVENTDEV_PMD'
 driver_name_fmt = 'rte_pmd_@0@_event'
-- 
2.11.0

[dpdk-dev] [PATCH v3 2/4] meson: change default config for armv8

[Yongseok Koh]

Current default cache line size for armv8 CPUs having Implementor ID of
0x41 is 128 bytes, changing it to 64 bytes. Also, the max number of lcores
is changed to 16 from 256.

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---

v3:
* decrease RTE_MAX_LCORE to 16 from 256
* change title and commit log

v2:
* introduce flags_arm replacing flags_generic instead of using the extra flags

 config/arm/meson.build | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/config/arm/meson.build b/config/arm/meson.build
index 22a062bad9..a5cce51707 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -32,6 +32,11 @@ flags_generic = [
 	['RTE_MAX_LCORE', 256],
 	['RTE_USE_C11_MEM_MODEL', true],
 	['RTE_CACHE_LINE_SIZE', 128]]
+flags_arm = [
+	['RTE_MACHINE', '"armv8a"'],
+	['RTE_MAX_LCORE', 16],
+	['RTE_USE_C11_MEM_MODEL', true],
+	['RTE_CACHE_LINE_SIZE', 64]]
 flags_cavium = [
 	['RTE_CACHE_LINE_SIZE', 128],
 	['RTE_MAX_NUMA_NODES', 2],
@@ -88,7 +93,7 @@ machine_args_cavium = [
 
 ## Arm implementer ID (ARM DDI 0487C.a, Section G7.2.106, Page G7-5321)
 impl_generic = ['Generic armv8', flags_generic, machine_args_generic]
-impl_0x41 = ['Arm', flags_generic, machine_args_generic]
+impl_0x41 = ['Arm', flags_arm, machine_args_generic]
 impl_0x42 = ['Broadcom', flags_generic, machine_args_generic]
 impl_0x43 = ['Cavium', flags_cavium, machine_args_cavium]
 impl_0x44 = ['DEC', flags_generic, machine_args_generic]
-- 
2.11.0

Re: [dpdk-dev] [PATCH v3 2/4] meson: change default config for armv8

[Honnappa Nagarahalli]

> 
> Current default cache line size for armv8 CPUs having Implementor ID of
> 0x41 is 128 bytes, changing it to 64 bytes. Also, the max number of lcores is
> changed to 16 from 256.
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
> 
> v3:
> * decrease RTE_MAX_LCORE to 16 from 256
> * change title and commit log
> 
> v2:
> * introduce flags_arm replacing flags_generic instead of using the extra flags
> 
>  config/arm/meson.build | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> 22a062bad9..a5cce51707 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -32,6 +32,11 @@ flags_generic = [
>  	['RTE_MAX_LCORE', 256],
>  	['RTE_USE_C11_MEM_MODEL', true],
>  	['RTE_CACHE_LINE_SIZE', 128]]
> +flags_arm = [
> +	['RTE_MACHINE', '"armv8a"'],
> +	['RTE_MAX_LCORE', 16],
> +	['RTE_USE_C11_MEM_MODEL', true],
> +	['RTE_CACHE_LINE_SIZE', 64]]
>  flags_cavium = [
>  	['RTE_CACHE_LINE_SIZE', 128],
>  	['RTE_MAX_NUMA_NODES', 2],
> @@ -88,7 +93,7 @@ machine_args_cavium = [
> 
>  ## Arm implementer ID (ARM DDI 0487C.a, Section G7.2.106, Page G7-5321)
> impl_generic = ['Generic armv8', flags_generic, machine_args_generic]
> -impl_0x41 = ['Arm', flags_generic, machine_args_generic]
> +impl_0x41 = ['Arm', flags_arm, machine_args_generic]
>  impl_0x42 = ['Broadcom', flags_generic, machine_args_generic]
>  impl_0x43 = ['Cavium', flags_cavium, machine_args_cavium]
>  impl_0x44 = ['DEC', flags_generic, machine_args_generic]
> --
Looks good.
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>

> 2.11.0

[dpdk-dev] [PATCH v3 3/4] net/mlx: fix library search in meson build

[Yongseok Koh]

If MLNX_OFED is installed, there's no .pc file installed for libraries and
dependency() can't find libraries by pkg-config. By adding fallback of
using cc.find_library(), libraries are properly located.

Fixes: e30b4e566f47 ("build: improve dependency handling")
Cc: bluca@debian.org
Cc: stable@dpdk.org

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---

v3:
* use lib name without 'lib' prefix for cc.find_library()

v2:
* change variable names to minimize code change

 drivers/net/mlx4/meson.build | 15 +++++++++------
 drivers/net/mlx5/meson.build | 15 +++++++++------
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/net/mlx4/meson.build b/drivers/net/mlx4/meson.build
index de020701d1..2540489bb7 100644
--- a/drivers/net/mlx4/meson.build
+++ b/drivers/net/mlx4/meson.build
@@ -13,14 +13,17 @@ if pmd_dlopen
 		'-DMLX4_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
 	]
 endif
-libs = [
-	dependency('libmnl', required:false),
-	dependency('libmlx4', required:false),
-	dependency('libibverbs', required:false),
-]
+libnames = [ 'mnl', 'mlx4', 'ibverbs' ]
+libs = []
 build = true
-foreach lib:libs
+foreach libname:libnames
+	lib = dependency('lib' + libname, required:false)
 	if not lib.found()
+		lib = cc.find_library(libname, required:false)
+	endif
+	if lib.found()
+		libs += [ lib ]
+	else
 		build = false
 	endif
 endforeach
diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
index a4c684e1b5..1a65c3a989 100644
--- a/drivers/net/mlx5/meson.build
+++ b/drivers/net/mlx5/meson.build
@@ -13,14 +13,17 @@ if pmd_dlopen
 		'-DMLX5_GLUE_VERSION="@0@"'.format(LIB_GLUE_VERSION),
 	]
 endif
-libs = [
-	dependency('libmnl', required:false),
-	dependency('libmlx5', required:false),
-	dependency('libibverbs', required:false),
-]
+libnames = [ 'mnl', 'mlx5', 'ibverbs' ]
+libs = []
 build = true
-foreach lib:libs
+foreach libname:libnames
+	lib = dependency('lib' + libname, required:false)
 	if not lib.found()
+		lib = cc.find_library(libname, required:false)
+	endif
+	if lib.found()
+		libs += [ lib ]
+	else
 		build = false
 	endif
 endforeach
-- 
2.11.0

Re: [dpdk-dev] [PATCH v3 3/4] net/mlx: fix library search in meson build

[Luca Boccassi]

On Thu, 2019-04-18 at 04:49 -0700, Yongseok Koh wrote:
> If MLNX_OFED is installed, there's no .pc file installed for
> libraries and
> dependency() can't find libraries by pkg-config. By adding fallback
> of
> using cc.find_library(), libraries are properly located.
> 
> Fixes: e30b4e566f47 ("build: improve dependency handling")
> Cc: 
> bluca@debian.org
> 
> Cc: 
> stable@dpdk.org
> 
> 
> Signed-off-by: Yongseok Koh <
> yskoh@mellanox.com
> >
> ---
> 
> v3:
> * use lib name without 'lib' prefix for cc.find_library()
> 
> v2:
> * change variable names to minimize code change
> 
>  drivers/net/mlx4/meson.build | 15 +++++++++------
>  drivers/net/mlx5/meson.build | 15 +++++++++------
>  2 files changed, 18 insertions(+), 12 deletions(-)

Acked-by: Luca Boccassi <bluca@debian.org>

-- 
Kind regards,
Luca Boccassi

[dpdk-dev] [PATCH v3 4/4] meson: add Mellanox BlueField cross-compile config

[Yongseok Koh]

Mellanox BlueField is armv8 CPU having cortex-a72. The implementor ID is
0x41 (arm) and the primary part number is 0xd08 (cortex-a72).

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
---
 config/arm/arm64_bluefield_linux_gcc | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 config/arm/arm64_bluefield_linux_gcc

diff --git a/config/arm/arm64_bluefield_linux_gcc b/config/arm/arm64_bluefield_linux_gcc
new file mode 100644
index 0000000000..304c4073d5
--- /dev/null
+++ b/config/arm/arm64_bluefield_linux_gcc
@@ -0,0 +1,16 @@
+[binaries]
+c = 'aarch64-linux-gnu-gcc'
+cpp = 'aarch64-linux-gnu-cpp'
+ar = 'aarch64-linux-gnu-gcc-ar'
+strip = 'aarch64-linux-gnu-strip'
+pcap-config = ''
+
+[host_machine]
+system = 'linux'
+cpu_family = 'aarch64'
+cpu = 'armv8-a'
+endian = 'little'
+
+[properties]
+implementor_id = '0x41'
+implementor_pn = '0xd08'
-- 
2.11.0

Re: [dpdk-dev] [PATCH v3 4/4] meson: add Mellanox BlueField cross-compile config

[Thomas Monjalon]

18/04/2019 13:49, Yongseok Koh:
> Mellanox BlueField is armv8 CPU having cortex-a72. The implementor ID is
> 0x41 (arm) and the primary part number is 0xd08 (cortex-a72).
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> Acked-by: Jerin Jacob <jerinj@marvell.com>

Series applied, thanks