[1/2] build: add option for armv8 crypto extension

Message ID 20190502015806.41497-1-yskoh@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [1/2] build: add option for armv8 crypto extension |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Yongseok Koh May 2, 2019, 1:58 a.m. UTC
  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(-)
  

Comments

Honnappa Nagarahalli May 2, 2019, 4:13 a.m. UTC | #1
> 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
  
Yongseok Koh May 2, 2019, 5:46 a.m. UTC | #2
> 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
>
  
Jerin Jacob Kollanukkaran May 2, 2019, 10 a.m. UTC | #3
> -----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
> >
  
Honnappa Nagarahalli May 3, 2019, 3:57 a.m. UTC | #4
> > 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
> >
  
Yongseok Koh May 3, 2019, 9:57 a.m. UTC | #5
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
> > >
>
  

Patch

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