[dpdk-dev] [PATCH 1/2] build: add option for armv8 crypto extension

[dpdk-dev] [PATCH 1/2] 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 7fa6ed3105..3b53842d08 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -8,6 +8,8 @@ march_opt = '-march=@0@'.format(machine)
 arm_force_native_march = false
 arm_force_default_march = (machine == 'default')
 
+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
@@ -74,14 +76,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']],
-	['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]],
+	['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.11.0

[dpdk-dev] [PATCH 2/2] 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.11.0

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

[Honnappa Nagarahalli]

> 
> 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
How do you plan to support the Bluefield devices with crypto enabled? Do you plan to add another config?

> +
>  # UMA architecture
>  CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n
>  CONFIG_RTE_LIBRTE_VHOST_NUMA=n
> --
> 2.11.0

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

[Yongseok Koh]

> On May 1, 2019, at 9:12 PM, Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
> 
>> 
>> 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
> How do you plan to support the Bluefield devices with crypto enabled? Do you plan to add another config?

Nope, user will have to enable it by modifying this file or specifying it in make command line.
This is just to provide the default which can make majority of cases work well.
For example, by default, mlx4/5 are even disabled in default x86 build config because
not all users have drv/lib installed on their system. Users have to enable it if they want.

>> +
>> # UMA architecture
>> CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n
>> CONFIG_RTE_LIBRTE_VHOST_NUMA=n
>> --
>> 2.11.0

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

[Honnappa Nagarahalli]

> > On May 1, 2019, at 9:12 PM, Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com> wrote:
> >
> >>
> >> 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
> > How do you plan to support the Bluefield devices with crypto enabled? Do
> you plan to add another config?
> 
> Nope, user will have to enable it by modifying this file or specifying it in make
> command line.
Ok, I suggest documenting it somewhere.

> This is just to provide the default which can make majority of cases work well.
> For example, by default, mlx4/5 are even disabled in default x86 build config
> because not all users have drv/lib installed on their system. Users have to
> enable it if they want.
> 
> >> +
> >> # UMA architecture
> >> CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES=n
> >> CONFIG_RTE_LIBRTE_VHOST_NUMA=n
> >> --
> >> 2.11.0

Re: [dpdk-dev] [PATCH 1/2] build: add option for armv8 crypto extension

[Honnappa Nagarahalli]

> 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.
The distro/binary portable image needs to be built without crypto. Only the crypto drivers need to be built with crypto and at run time we need to hook up the correct function pointers. So, IMO, by default crypto should be disabled and should be enabled in specific target machine configs. 

> 
> 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
> 7fa6ed3105..3b53842d08 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -8,6 +8,8 @@ march_opt = '-march=@0@'.format(machine)
> arm_force_native_march = false  arm_force_default_march = (machine ==
> 'default')
> 
> +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 @@ -74,14 +76,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']],
> -	['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]],
> +	['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.11.0

Re: [dpdk-dev] [PATCH 1/2] build: add option for armv8 crypto extension

[Yongseok Koh]

> On May 1, 2019, at 9:13 PM, Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
> 
>> 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.
> The distro/binary portable image needs to be built without crypto. Only the crypto drivers need to be built with crypto and at run time we need to hook up the correct function pointers. So, IMO, by default crypto should be disabled and should be enabled in specific target machine configs.

I make it enabled by default simply because I don't want to change the current
behavior, no breakage.

I also want to hear from others. Jerin, Thomas?

>> 
>> 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
>> 7fa6ed3105..3b53842d08 100644
>> --- a/config/arm/meson.build
>> +++ b/config/arm/meson.build
>> @@ -8,6 +8,8 @@ march_opt = '-march=@0@'.format(machine)
>> arm_force_native_march = false  arm_force_default_march = (machine ==
>> 'default')
>> 
>> +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 @@ -74,14 +76,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']],
>> -	['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]],
>> +	['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.11.0
> 

Re: [dpdk-dev] [PATCH 1/2] build: add option for armv8 crypto extension

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Yongseok Koh
> Sent: Thursday, May 2, 2019 11:17 AM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Shahaf Shuler
> <shahafs@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>;
> dev@dpdk.org; bruce.richardson@intel.com; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>; Gavin Hu (Arm Technology China)
> <Gavin.Hu@arm.com>; nd <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH 1/2] build: add option for armv8 crypto
> extension
> 
> > On May 1, 2019, at 9:13 PM, Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com> wrote:
> >
> >> 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.
> > The distro/binary portable image needs to be built without crypto. Only the
> crypto drivers need to be built with crypto and at run time we need to hook up
> the correct function pointers. So, IMO, by default crypto should be disabled and
> should be enabled in specific target machine configs.
> 
> I make it enabled by default simply because I don't want to change the current
> behavior, no breakage.
> 
> I also want to hear from others. Jerin, Thomas?

I think, Honnapa suggestion would work as core crypto driver is build as separate binary now.
https://github.com/caviumnetworks/armv8_crypto	
If so, It makes sense to disable crypto by default for DPDK as DPDK directly not using any
crypto instructions.






> 
> >>
> >> 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
> >> 7fa6ed3105..3b53842d08 100644
> >> --- a/config/arm/meson.build
> >> +++ b/config/arm/meson.build
> >> @@ -8,6 +8,8 @@ march_opt = '-march=@0@'.format(machine)
> >> arm_force_native_march = false  arm_force_default_march = (machine ==
> >> 'default')
> >>
> >> +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 @@ -74,14 +76,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']],
> >> -	['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]],
> >> +	['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.11.0
> >

Re: [dpdk-dev] [PATCH 1/2] build: add option for armv8 crypto extension

[Honnappa Nagarahalli]

> > On May 1, 2019, at 9:13 PM, Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com> wrote:
> >
> >> 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.
> > The distro/binary portable image needs to be built without crypto. Only the
> crypto drivers need to be built with crypto and at run time we need to hook
> up the correct function pointers. So, IMO, by default crypto should be
> disabled and should be enabled in specific target machine configs.
> 
> I make it enabled by default simply because I don't want to change the
> current behavior, no breakage.
Good point. I don't know how to handle it either. But, the current distro package will not work on BlueField as it is built with crypto enabled. So, IMO, we should consider it as bug.

> 
> I also want to hear from others. Jerin, Thomas?
> 
> >>
> >> 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
> >> 7fa6ed3105..3b53842d08 100644
> >> --- a/config/arm/meson.build
> >> +++ b/config/arm/meson.build
> >> @@ -8,6 +8,8 @@ march_opt = '-march=@0@'.format(machine)
> >> arm_force_native_march = false  arm_force_default_march = (machine ==
> >> 'default')
> >>
> >> +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 @@ -74,14 +76,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']],
> >> -	['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]],
> >> +	['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.11.0
> >

Re: [dpdk-dev] [PATCH 1/2] build: add option for armv8 crypto extension

[Yongseok Koh]

On Fri, May 03, 2019 at 03:57:20AM +0000, Honnappa Nagarahalli wrote:
> > > On May 1, 2019, at 9:13 PM, Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com> wrote:
> > >
> > >> 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.
> > > The distro/binary portable image needs to be built without crypto. Only the
> > crypto drivers need to be built with crypto and at run time we need to hook
> > up the correct function pointers. So, IMO, by default crypto should be
> > disabled and should be enabled in specific target machine configs.
> > 
> > I make it enabled by default simply because I don't want to change the
> > current behavior, no breakage.
> Good point. I don't know how to handle it either. But, the current distro
> package will not work on BlueField as it is built with crypto enabled. So,
> IMO, we should consider it as bug.

You may probably know, cpu w/o crypto is related to the export licenses.
Sometimes, vendor has to disable it when exporting in certain conditions. And as
BlueField is still in early stage, we don't mind much existing distro packages
not being able to run on such parts (BlueField having no crypto).

> > I also want to hear from others. Jerin, Thomas?
> > 
> > >>
> > >> 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
> > >> 7fa6ed3105..3b53842d08 100644
> > >> --- a/config/arm/meson.build
> > >> +++ b/config/arm/meson.build
> > >> @@ -8,6 +8,8 @@ march_opt = '-march=@0@'.format(machine)
> > >> arm_force_native_march = false  arm_force_default_march = (machine ==
> > >> 'default')
> > >>
> > >> +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 @@ -74,14 +76,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']],
> > >> -	['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]],
> > >> +	['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.11.0
> > >
> 

[dpdk-dev] [PATCH v2] build: disable 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.

It is 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.

'+crypto' flag currently implies only '+aes' and '+sha2' and enabling it
will generate the crypto instructions only when crypto intrinsics are used.
For the devices supporting 8.2 crypto or newer, compiler could generate
such instructions beyond intrinsics or asm code. For example, compiler can
generate 3-way exclusive OR instructions if sha3 is supported. However, it
has to be enabled by adding '+sha3' as of today.

In DPDK, armv8 cryptodev is the only one which requires the crypto support.
As it even uses external library of Marvell which is compiled out of DPDK
with crypto support and there's run-time check for required cpuflags,
crypto support can be disabled in DPDK.

Cc: stable@dpdk.org

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

v2:
* disable crypto support instead of having a build config

 config/arm/meson.build        | 2 +-
 mk/machine/armv8a/rte.vars.mk | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

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

Re: [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension

[Honnappa Nagarahalli]

Hi Yongseok,
	We need to enable 'CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO' (which would require a documentation change in [1]). I think this change might have an impact on the existing users. Does this change need to be documented somewhere (at least in the release notes)?

[1] https://doc.dpdk.org/guides-19.02/cryptodevs/armv8.html

> -----Original Message-----
> From: Yongseok Koh <yskoh@mellanox.com>
> Sent: Friday, May 3, 2019 7:28 AM
> To: jerinj@marvell.com; thomas@monjalon.net
> Cc: dev@dpdk.org; bruce.richardson@intel.com; pbhagavatula@marvell.com;
> shahafs@mellanox.com; Gavin Hu (Arm Technology China)
> <Gavin.Hu@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; stable@dpdk.org
> Subject: [PATCH v2] build: disable armv8 crypto extension
> 
> 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.
> 
> It is 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.
> 
> '+crypto' flag currently implies only '+aes' and '+sha2' and enabling it will
> generate the crypto instructions only when crypto intrinsics are used.
> For the devices supporting 8.2 crypto or newer, compiler could generate such
> instructions beyond intrinsics or asm code. For example, compiler can
> generate 3-way exclusive OR instructions if sha3 is supported. However, it
> has to be enabled by adding '+sha3' as of today.
> 
> In DPDK, armv8 cryptodev is the only one which requires the crypto support.
> As it even uses external library of Marvell which is compiled out of DPDK with
> crypto support and there's run-time check for required cpuflags, crypto
> support can be disabled in DPDK.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
> 
> v2:
> * disable crypto support instead of having a build config
> 
>  config/arm/meson.build        | 2 +-
>  mk/machine/armv8a/rte.vars.mk | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> 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']],
IIRC, this would impact distro packaging as well. Adding Luca.

>  	['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
> --
> 2.11.0

Re: [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension

[Honnappa Nagarahalli]

> 
> Hi Yongseok,
> 	We need to enable 'CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO'
> (which would require a documentation change in [1]).
I enabled this and compiled, the compilation fails. Ideally (as discussed in other threads), the PMD code itself does not make use of the crypto instructions, so we should be able to compile the PMDs. Crypto functionality should not be available only if crypto is not available from the CPU or the crypto library is not present. Otherwise, there is no way to use crypto in distro package without recompiling. We can take this up separately. 

 I think this change might
> have an impact on the existing users. Does this change need to be documented
> somewhere (at least in the release notes)?
> 
> [1] https://doc.dpdk.org/guides-19.02/cryptodevs/armv8.html
> 
> > -----Original Message-----
> > From: Yongseok Koh <yskoh@mellanox.com>
> > Sent: Friday, May 3, 2019 7:28 AM
> > To: jerinj@marvell.com; thomas@monjalon.net
> > Cc: dev@dpdk.org; bruce.richardson@intel.com;
> > pbhagavatula@marvell.com; shahafs@mellanox.com; Gavin Hu (Arm
> > Technology China) <Gavin.Hu@arm.com>; Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com>; stable@dpdk.org
> > Subject: [PATCH v2] build: disable armv8 crypto extension
> >
> > 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.
> >
> > It is 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.
> >
> > '+crypto' flag currently implies only '+aes' and '+sha2' and enabling
> > it will generate the crypto instructions only when crypto intrinsics are used.
> > For the devices supporting 8.2 crypto or newer, compiler could
> > generate such instructions beyond intrinsics or asm code. For example,
> > compiler can generate 3-way exclusive OR instructions if sha3 is
> > supported. However, it has to be enabled by adding '+sha3' as of today.
> >
> > In DPDK, armv8 cryptodev is the only one which requires the crypto support.
> > As it even uses external library of Marvell which is compiled out of
> > DPDK with crypto support and there's run-time check for required
> > cpuflags, crypto support can be disabled in DPDK.
> >
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > ---
> >
> > v2:
> > * disable crypto support instead of having a build config
> >
> >  config/arm/meson.build        | 2 +-
> >  mk/machine/armv8a/rte.vars.mk | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > 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']],
> IIRC, this would impact distro packaging as well. Adding Luca.
> 
> >  	['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
> > --
> > 2.11.0

Re: [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension

[Yongseok Koh]

On Fri, May 03, 2019 at 04:10:47PM +0000, Honnappa Nagarahalli wrote:
> > 
> > Hi Yongseok,
> > 	We need to enable 'CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO'
> > (which would require a documentation change in [1]).
> I enabled this and compiled, the compilation fails. Ideally (as discussed in
> other threads), the PMD code itself does not make use of the crypto
> instructions, so we should be able to compile the PMDs. Crypto functionality
> should not be available only if crypto is not available from the CPU or the
> crypto library is not present. Otherwise, there is no way to use crypto in
> distro package without recompiling. We can take this up separately. 

Like you mentioned, the failure isn't due to lack of '+crypto' flag but the
external library.

CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO is disabled by default because it needs the
external library. In order to enable the config, ARMV8_CRYPTO_LIB_PATH must be
specified. Makefile mandates it.

	ifneq ($(MAKECMDGOALS),clean)
	ifneq ($(MAKECMDGOALS),config)
	ifeq ($(ARMV8_CRYPTO_LIB_PATH),)
	$(error "Please define ARMV8_CRYPTO_LIB_PATH environment variable")
	endif
	endif
	endif

armv8 crypto pmd has nothing to do with '+crypto' cpuflag but the external
library (compiled with '+crypto' flag) has to be linked. Without the external
library, it is meaningless to compile the armv8 crypto PMD because we don't have
any fallback.

In runtime, in order to guarantee the final binary runs w/o crash, it checks
cpuflag of the target host with rte_cpu_get_flag_enabled().

>  I think this change might have an impact on the existing users. Does this
>  change need to be documented somewhere (at least in the release notes)?

Practically, there would be no impact. Even if user's app makes use of crypto
instructions, the build config of the app should have the flag.

> > 
> > [1] https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdoc.dpdk.org%2Fguides-19.02%2Fcryptodevs%2Farmv8.html&data=02%7C01%7Cyskoh%40mellanox.com%7C4d6fc9819e4e4a5b1ba508d6cfe1e91d%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636924966530971290&sdata=r3VFKjX44T1g4ORlibYCuCpCDQl1BPQyhzfEe%2BGz%2Fy8%3D&reserved=0
> > 
> > > -----Original Message-----
> > > From: Yongseok Koh <yskoh@mellanox.com>
> > > Sent: Friday, May 3, 2019 7:28 AM
> > > To: jerinj@marvell.com; thomas@monjalon.net
> > > Cc: dev@dpdk.org; bruce.richardson@intel.com;
> > > pbhagavatula@marvell.com; shahafs@mellanox.com; Gavin Hu (Arm
> > > Technology China) <Gavin.Hu@arm.com>; Honnappa Nagarahalli
> > > <Honnappa.Nagarahalli@arm.com>; stable@dpdk.org
> > > Subject: [PATCH v2] build: disable armv8 crypto extension
> > >
> > > 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.
> > >
> > > It is 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.
> > >
> > > '+crypto' flag currently implies only '+aes' and '+sha2' and enabling
> > > it will generate the crypto instructions only when crypto intrinsics are used.
> > > For the devices supporting 8.2 crypto or newer, compiler could
> > > generate such instructions beyond intrinsics or asm code. For example,
> > > compiler can generate 3-way exclusive OR instructions if sha3 is
> > > supported. However, it has to be enabled by adding '+sha3' as of today.
> > >
> > > In DPDK, armv8 cryptodev is the only one which requires the crypto support.
> > > As it even uses external library of Marvell which is compiled out of
> > > DPDK with crypto support and there's run-time check for required
> > > cpuflags, crypto support can be disabled in DPDK.
> > >
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > > ---
> > >
> > > v2:
> > > * disable crypto support instead of having a build config
> > >
> > >  config/arm/meson.build        | 2 +-
> > >  mk/machine/armv8a/rte.vars.mk | 2 +-
> > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > 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']],
> > IIRC, this would impact distro packaging as well. Adding Luca.
> > 
> > >  	['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
> > > --
> > > 2.11.0
> 

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] build: disable armv8 crypto extension

[Yongseok Koh]

> On May 3, 2019, at 10:50 AM, Yongseok Koh <yskoh@mellanox.com> wrote:
> 
>> I think this change might have an impact on the existing users. Does this
>> change need to be documented somewhere (at least in the release notes)?
> 
> Practically, there would be no impact. Even if user's app makes use of crypto
> instructions, the build config of the app should have the flag.

On IRC, I could see you concerned about the case where user app derives build
flags from DPDK like apps under the examples directory. Yes, that's a valid
concern.

Thomas/Jerin, where could be the best spot to document it? Just release note?
I think release note would be fine.

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] build: disable armv8 crypto extension

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Yongseok Koh <yskoh@mellanox.com>
> Sent: Tuesday, May 7, 2019 3:17 AM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Thomas Monjalon
> <thomas@monjalon.net>; dev@dpdk.org; bruce.richardson@intel.com;
> Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Shahaf Shuler
> <shahafs@mellanox.com>; Gavin Hu (Arm Technology China)
> <Gavin.Hu@arm.com>; stable@dpdk.org; Luca Boccassi
> <bluca@debian.org>; nd <nd@arm.com>
> Subject: [EXT] Re: [dpdk-stable] [PATCH v2] build: disable armv8 crypto
> extension
> 
> > On May 3, 2019, at 10:50 AM, Yongseok Koh <yskoh@mellanox.com>
> wrote:
> >
> >> I think this change might have an impact on the existing users. Does
> >> this change need to be documented somewhere (at least in the release
> notes)?
> >
> > Practically, there would be no impact. Even if user's app makes use of
> > crypto instructions, the build config of the app should have the flag.
> 
> On IRC, I could see you concerned about the case where user app derives
> build flags from DPDK like apps under the examples directory. Yes, that's a
> valid concern.

Is there any DPDK example application would depend on the armv8 crypto flags? IMO, None of
the applications are directly using armv8 crypto instruction. Even such as comes for any external DPDK
app which is not in dpdk.org tree then it can be added in APP makefile.
IMO, app writes should be aware of the need for using +crypto if he/she using the crypto instruction.

> 
> Thomas/Jerin, where could be the best spot to document it? Just release
> note?
> I think release note would be fine.

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] build: disable armv8 crypto extension

[Honnappa Nagarahalli]

> > Subject: [EXT] Re: [dpdk-stable] [PATCH v2] build: disable armv8
> > crypto extension
> >
> > > On May 3, 2019, at 10:50 AM, Yongseok Koh <yskoh@mellanox.com>
> > wrote:
> > >
> > >> I think this change might have an impact on the existing users.
> > >> Does this change need to be documented somewhere (at least in the
> > >> release
> > notes)?
> > >
> > > Practically, there would be no impact. Even if user's app makes use
> > > of crypto instructions, the build config of the app should have the flag.
> >
> > On IRC, I could see you concerned about the case where user app
> > derives build flags from DPDK like apps under the examples directory.
> > Yes, that's a valid concern.
> 
> Is there any DPDK example application would depend on the armv8 crypto
> flags? IMO, None of the applications are directly using armv8 crypto
It is not about DPDK's example application. We have control over that.

> instruction. Even such as comes for any external DPDK app which is not in
> dpdk.org tree then it can be added in APP makefile.
> IMO, app writes should be aware of the need for using +crypto if he/she
> using the crypto instruction.
Agree. But, it might be that the app writer might have depended on the +crypto coming from DPDK build system, in which case, she/he needs to know the change through release notes?

I do not think it needs to be documented anywhere else.

> 
> >
> > Thomas/Jerin, where could be the best spot to document it? Just
> > release note?
> > I think release note would be fine.
> 

Re: [dpdk-dev] [dpdk-stable] [PATCH v2] build: disable armv8 crypto extension

[Jerin Jacob Kollanukkaran]

> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Tuesday, May 7, 2019 4:34 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; yskoh@mellanox.com
> Cc: thomas@monjalon.net; dev@dpdk.org; bruce.richardson@intel.com; Pavan
> Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Shahaf Shuler
> <shahafs@mellanox.com>; Gavin Hu (Arm Technology China)
> <Gavin.Hu@arm.com>; stable@dpdk.org; Luca Boccassi <bluca@debian.org>;
> nd <nd@arm.com>; nd <nd@arm.com>
> Subject: [EXT] RE: [dpdk-stable] [PATCH v2] build: disable armv8 crypto
> extension
> 
> > > Subject: [EXT] Re: [dpdk-stable] [PATCH v2] build: disable armv8
> > > crypto extension
> > >
> > > > On May 3, 2019, at 10:50 AM, Yongseok Koh <yskoh@mellanox.com>
> > > wrote:
> > > >
> > > >> I think this change might have an impact on the existing users.
> > > >> Does this change need to be documented somewhere (at least in the
> > > >> release
> > > notes)?
> > > >
> > > > Practically, there would be no impact. Even if user's app makes
> > > > use of crypto instructions, the build config of the app should have the flag.
> > >
> > > On IRC, I could see you concerned about the case where user app
> > > derives build flags from DPDK like apps under the examples directory.
> > > Yes, that's a valid concern.
> >
> > Is there any DPDK example application would depend on the armv8 crypto
> > flags? IMO, None of the applications are directly using armv8 crypto
> It is not about DPDK's example application. We have control over that.
> 
> > instruction. Even such as comes for any external DPDK app which is not
> > in dpdk.org tree then it can be added in APP makefile.
> > IMO, app writes should be aware of the need for using +crypto if
> > he/she using the crypto instruction.
> Agree. But, it might be that the app writer might have depended on the +crypto
> coming from DPDK build system, in which case, she/he needs to know the
> change through release notes?
> 
> I do not think it needs to be documented anywhere else.

Agree and no harm in updating the release notes.

> 
> >
> > >
> > > Thomas/Jerin, where could be the best spot to document it? Just
> > > release note?
> > > I think release note would be fine.
> >

Re: [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension

[Dharmik Thakkar]

Hi Yongseok,
Thank you for the patch! I have tested it on Bluefield reference platform. It works fine. 

One observation:
Even though I used 'make config T=arm64-bluefield-linuxapp-gcc’,
at the end of the build process it said 'Build complete [arm64-armv8a-linuxapp-gcc]’

Tested-by: Dharmik Thakkar <dharmik.thakkar@arm.com>

> On May 3, 2019, at 7:28 AM, Yongseok Koh <yskoh@mellanox.com> wrote:
> 
> 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.
> 
> It is 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.
> 
> '+crypto' flag currently implies only '+aes' and '+sha2' and enabling it
> will generate the crypto instructions only when crypto intrinsics are used.
> For the devices supporting 8.2 crypto or newer, compiler could generate
> such instructions beyond intrinsics or asm code. For example, compiler can
> generate 3-way exclusive OR instructions if sha3 is supported. However, it
> has to be enabled by adding '+sha3' as of today.
> 
> In DPDK, armv8 cryptodev is the only one which requires the crypto support.
> As it even uses external library of Marvell which is compiled out of DPDK
> with crypto support and there's run-time check for required cpuflags,
> crypto support can be disabled in DPDK.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
> 
> v2:
> * disable crypto support instead of having a build config
> 
> config/arm/meson.build        | 2 +-
> mk/machine/armv8a/rte.vars.mk | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
> 
> 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
> -- 
> 2.11.0
> 

Re: [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension

[Yongseok Koh]

> On May 3, 2019, at 3:13 PM, Dharmik Thakkar <Dharmik.Thakkar@arm.com> wrote:
> 
> Hi Yongseok,
> Thank you for the patch! I have tested it on Bluefield reference platform. It works fine. 

Thanks for your testing.

> One observation:
> Even though I used 'make config T=arm64-bluefield-linuxapp-gcc’,
> at the end of the build process it said 'Build complete [arm64-armv8a-linuxapp-gcc]’

The BlueField config stems from armv8a.
Unlike Cavium hosts, RTE_MACHINE is still armv8a.
Looks it is same for Broadcom's Stingray.

> Tested-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> 
>> On May 3, 2019, at 7:28 AM, Yongseok Koh <yskoh@mellanox.com> wrote:
>> 
>> 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.
>> 
>> It is 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.
>> 
>> '+crypto' flag currently implies only '+aes' and '+sha2' and enabling it
>> will generate the crypto instructions only when crypto intrinsics are used.
>> For the devices supporting 8.2 crypto or newer, compiler could generate
>> such instructions beyond intrinsics or asm code. For example, compiler can
>> generate 3-way exclusive OR instructions if sha3 is supported. However, it
>> has to be enabled by adding '+sha3' as of today.
>> 
>> In DPDK, armv8 cryptodev is the only one which requires the crypto support.
>> As it even uses external library of Marvell which is compiled out of DPDK
>> with crypto support and there's run-time check for required cpuflags,
>> crypto support can be disabled in DPDK.
>> 
>> Cc: stable@dpdk.org
>> 
>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>> ---
>> 
>> v2:
>> * disable crypto support instead of having a build config
>> 
>> config/arm/meson.build        | 2 +-
>> mk/machine/armv8a/rte.vars.mk | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>> 
>> 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
>> -- 
>> 2.11.0
>> 
> 

[dpdk-dev] [PATCH v3] build: disable 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.

It is 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.

'+crypto' flag currently implies only '+aes' and '+sha2' and enabling it
will generate the crypto instructions only when crypto intrinsics are used.
For the devices supporting 8.2 crypto or newer, compiler could generate
such instructions beyond intrinsics or asm code. For example, compiler can
generate 3-way exclusive OR instructions if sha3 is supported. However, it
has to be enabled by adding '+sha3' as of today.

In DPDK, armv8 cryptodev is the only one which requires the crypto support.
As it even uses external library of Marvell which is compiled out of DPDK
with crypto support and there's run-time check for required cpuflags,
crypto support can be disabled in DPDK.

Cc: stable@dpdk.org

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
Tested-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
---

v3:
* announce removal of the flag in release note

v2:
* disable crypto support instead of having a build config

 config/arm/meson.build                 | 2 +-
 doc/guides/rel_notes/release_19_05.rst | 2 ++
 mk/machine/armv8a/rte.vars.mk          | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

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/doc/guides/rel_notes/release_19_05.rst b/doc/guides/rel_notes/release_19_05.rst
index 5044ac7df1..0d35f25094 100644
--- a/doc/guides/rel_notes/release_19_05.rst
+++ b/doc/guides/rel_notes/release_19_05.rst
@@ -234,6 +234,8 @@ Removed Items
    Also, make sure to start the actual text at the margin.
    =========================================================
 
+* build: armv8 crypto extension is disabled.
+
 
 API Changes
 -----------
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
-- 
2.21.0

Re: [dpdk-dev] [PATCH v3] build: disable armv8 crypto extension

[Honnappa Nagarahalli]

> 
> 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.
> 
> It is 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.
> 
> '+crypto' flag currently implies only '+aes' and '+sha2' and enabling it will
> generate the crypto instructions only when crypto intrinsics are used.
> For the devices supporting 8.2 crypto or newer, compiler could generate such
> instructions beyond intrinsics or asm code. For example, compiler can generate
> 3-way exclusive OR instructions if sha3 is supported. However, it has to be
> enabled by adding '+sha3' as of today.
> 
> In DPDK, armv8 cryptodev is the only one which requires the crypto support.
> As it even uses external library of Marvell which is compiled out of DPDK with
> crypto support and there's run-time check for required cpuflags, crypto
> support can be disabled in DPDK.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> Acked-by: Jerin Jacob <jerinj@marvell.com>
> Tested-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> ---
> 
> v3:
> * announce removal of the flag in release note
> 
> v2:
> * disable crypto support instead of having a build config
> 
>  config/arm/meson.build                 | 2 +-
>  doc/guides/rel_notes/release_19_05.rst | 2 ++
>  mk/machine/armv8a/rte.vars.mk          | 2 +-
>  3 files changed, 4 insertions(+), 2 deletions(-)
> 
> 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/doc/guides/rel_notes/release_19_05.rst
> b/doc/guides/rel_notes/release_19_05.rst
> index 5044ac7df1..0d35f25094 100644
> --- a/doc/guides/rel_notes/release_19_05.rst
> +++ b/doc/guides/rel_notes/release_19_05.rst
> @@ -234,6 +234,8 @@ Removed Items
>     Also, make sure to start the actual text at the margin.
>     =========================================================
> 
> +* build: armv8 crypto extension is disabled.
> +
> 
>  API Changes
>  -----------
> 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
> --
> 2.21.0

Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>

Re: [dpdk-dev] [PATCH v3] build: disable armv8 crypto extension

[Thomas Monjalon]

13/05/2019 21:26, Honnappa Nagarahalli:
> > 
> > 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.
> > 
> > It is 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.
> > 
> > '+crypto' flag currently implies only '+aes' and '+sha2' and enabling it will
> > generate the crypto instructions only when crypto intrinsics are used.
> > For the devices supporting 8.2 crypto or newer, compiler could generate such
> > instructions beyond intrinsics or asm code. For example, compiler can generate
> > 3-way exclusive OR instructions if sha3 is supported. However, it has to be
> > enabled by adding '+sha3' as of today.
> > 
> > In DPDK, armv8 cryptodev is the only one which requires the crypto support.
> > As it even uses external library of Marvell which is compiled out of DPDK with
> > crypto support and there's run-time check for required cpuflags, crypto
> > support can be disabled in DPDK.
> > 
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > Acked-by: Jerin Jacob <jerinj@marvell.com>
> > Tested-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> 
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>

Applied, thanks