DPDK patches and discussions
 help / color / mirror / Atom feed
* Re: [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension
@ 2019-05-03 16:06 Jerin Jacob Kollanukkaran
  2019-05-03 16:06 ` Jerin Jacob Kollanukkaran
  0 siblings, 1 reply; 14+ messages in thread
From: Jerin Jacob Kollanukkaran @ 2019-05-03 16:06 UTC (permalink / raw)
  To: Yongseok Koh, thomas
  Cc: dev, bruce.richardson, Pavan Nikhilesh Bhagavatula, shahafs,
	gavin.hu, Honnappa.Nagarahalli, stable

> -----Original Message-----
> From: Yongseok Koh <yskoh@mellanox.com>
> Sent: Friday, May 3, 2019 5:58 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; thomas@monjalon.net
> Cc: dev@dpdk.org; bruce.richardson@intel.com; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>; shahafs@mellanox.com; gavin.hu@arm.com;
> Honnappa.Nagarahalli@arm.com; stable@dpdk.org
> Subject: [EXT] [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>
> ---

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension
  2019-05-03 16:06 [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension Jerin Jacob Kollanukkaran
@ 2019-05-03 16:06 ` Jerin Jacob Kollanukkaran
  0 siblings, 0 replies; 14+ messages in thread
From: Jerin Jacob Kollanukkaran @ 2019-05-03 16:06 UTC (permalink / raw)
  To: Yongseok Koh, thomas
  Cc: dev, bruce.richardson, Pavan Nikhilesh Bhagavatula, shahafs,
	gavin.hu, Honnappa.Nagarahalli, stable

> -----Original Message-----
> From: Yongseok Koh <yskoh@mellanox.com>
> Sent: Friday, May 3, 2019 5:58 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; thomas@monjalon.net
> Cc: dev@dpdk.org; bruce.richardson@intel.com; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>; shahafs@mellanox.com; gavin.hu@arm.com;
> Honnappa.Nagarahalli@arm.com; stable@dpdk.org
> Subject: [EXT] [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>
> ---

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension
  2019-05-03 22:13   ` Dharmik Thakkar
  2019-05-03 22:13     ` Dharmik Thakkar
@ 2019-05-06 21:41     ` Yongseok Koh
  2019-05-06 21:41       ` Yongseok Koh
  1 sibling, 1 reply; 14+ messages in thread
From: Yongseok Koh @ 2019-05-06 21:41 UTC (permalink / raw)
  To: Dharmik Thakkar
  Cc: jerinj, Thomas Monjalon, dev, Bruce Richardson, pbhagavatula,
	Shahaf Shuler, Gavin Hu (Arm Technology China),
	Honnappa Nagarahalli, stable, nd


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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension
  2019-05-06 21:41     ` Yongseok Koh
@ 2019-05-06 21:41       ` Yongseok Koh
  0 siblings, 0 replies; 14+ messages in thread
From: Yongseok Koh @ 2019-05-06 21:41 UTC (permalink / raw)
  To: Dharmik Thakkar
  Cc: jerinj, Thomas Monjalon, dev, Bruce Richardson, pbhagavatula,
	Shahaf Shuler, Gavin Hu (Arm Technology China),
	Honnappa Nagarahalli, stable, nd


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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension
  2019-05-03 12:28 ` [dpdk-dev] [PATCH v2] build: disable " Yongseok Koh
  2019-05-03 12:28   ` Yongseok Koh
  2019-05-03 15:02   ` Honnappa Nagarahalli
@ 2019-05-03 22:13   ` Dharmik Thakkar
  2019-05-03 22:13     ` Dharmik Thakkar
  2019-05-06 21:41     ` Yongseok Koh
  2 siblings, 2 replies; 14+ messages in thread
From: Dharmik Thakkar @ 2019-05-03 22:13 UTC (permalink / raw)
  To: yskoh
  Cc: jerinj, thomas, dev, Bruce Richardson, pbhagavatula, shahafs,
	Gavin Hu (Arm Technology China),
	Honnappa Nagarahalli, stable, nd

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
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension
  2019-05-03 22:13   ` Dharmik Thakkar
@ 2019-05-03 22:13     ` Dharmik Thakkar
  2019-05-06 21:41     ` Yongseok Koh
  1 sibling, 0 replies; 14+ messages in thread
From: Dharmik Thakkar @ 2019-05-03 22:13 UTC (permalink / raw)
  To: yskoh
  Cc: jerinj, thomas, dev, Bruce Richardson, pbhagavatula, shahafs,
	Gavin Hu (Arm Technology China),
	Honnappa Nagarahalli, stable, nd

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
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension
  2019-05-03 16:10     ` Honnappa Nagarahalli
  2019-05-03 16:10       ` Honnappa Nagarahalli
@ 2019-05-03 17:50       ` Yongseok Koh
  2019-05-03 17:50         ` Yongseok Koh
  1 sibling, 1 reply; 14+ messages in thread
From: Yongseok Koh @ 2019-05-03 17:50 UTC (permalink / raw)
  To: Honnappa Nagarahalli
  Cc: jerinj, Thomas Monjalon, dev, bruce.richardson, pbhagavatula,
	Shahaf Shuler, Gavin Hu (Arm Technology China),
	stable, Luca Boccassi, nd

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&amp;data=02%7C01%7Cyskoh%40mellanox.com%7C4d6fc9819e4e4a5b1ba508d6cfe1e91d%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636924966530971290&amp;sdata=r3VFKjX44T1g4ORlibYCuCpCDQl1BPQyhzfEe%2BGz%2Fy8%3D&amp;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
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension
  2019-05-03 17:50       ` Yongseok Koh
@ 2019-05-03 17:50         ` Yongseok Koh
  0 siblings, 0 replies; 14+ messages in thread
From: Yongseok Koh @ 2019-05-03 17:50 UTC (permalink / raw)
  To: Honnappa Nagarahalli
  Cc: jerinj, Thomas Monjalon, dev, bruce.richardson, pbhagavatula,
	Shahaf Shuler, Gavin Hu (Arm Technology China),
	stable, Luca Boccassi, nd

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&amp;data=02%7C01%7Cyskoh%40mellanox.com%7C4d6fc9819e4e4a5b1ba508d6cfe1e91d%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636924966530971290&amp;sdata=r3VFKjX44T1g4ORlibYCuCpCDQl1BPQyhzfEe%2BGz%2Fy8%3D&amp;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
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension
  2019-05-03 15:02   ` Honnappa Nagarahalli
  2019-05-03 15:02     ` Honnappa Nagarahalli
@ 2019-05-03 16:10     ` Honnappa Nagarahalli
  2019-05-03 16:10       ` Honnappa Nagarahalli
  2019-05-03 17:50       ` Yongseok Koh
  1 sibling, 2 replies; 14+ messages in thread
From: Honnappa Nagarahalli @ 2019-05-03 16:10 UTC (permalink / raw)
  To: yskoh, jerinj, thomas
  Cc: dev, bruce.richardson, pbhagavatula, shahafs,
	Gavin Hu (Arm Technology China),
	stable, Luca Boccassi, Honnappa Nagarahalli, nd, nd

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension
  2019-05-03 16:10     ` Honnappa Nagarahalli
@ 2019-05-03 16:10       ` Honnappa Nagarahalli
  2019-05-03 17:50       ` Yongseok Koh
  1 sibling, 0 replies; 14+ messages in thread
From: Honnappa Nagarahalli @ 2019-05-03 16:10 UTC (permalink / raw)
  To: yskoh, jerinj, thomas
  Cc: dev, bruce.richardson, pbhagavatula, shahafs,
	Gavin Hu (Arm Technology China),
	stable, Luca Boccassi, Honnappa Nagarahalli, nd, nd

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension
  2019-05-03 12:28 ` [dpdk-dev] [PATCH v2] build: disable " Yongseok Koh
  2019-05-03 12:28   ` Yongseok Koh
@ 2019-05-03 15:02   ` Honnappa Nagarahalli
  2019-05-03 15:02     ` Honnappa Nagarahalli
  2019-05-03 16:10     ` Honnappa Nagarahalli
  2019-05-03 22:13   ` Dharmik Thakkar
  2 siblings, 2 replies; 14+ messages in thread
From: Honnappa Nagarahalli @ 2019-05-03 15:02 UTC (permalink / raw)
  To: yskoh, jerinj, thomas
  Cc: dev, bruce.richardson, pbhagavatula, shahafs,
	Gavin Hu (Arm Technology China),
	stable, Luca Boccassi, Honnappa Nagarahalli, nd, nd

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension
  2019-05-03 15:02   ` Honnappa Nagarahalli
@ 2019-05-03 15:02     ` Honnappa Nagarahalli
  2019-05-03 16:10     ` Honnappa Nagarahalli
  1 sibling, 0 replies; 14+ messages in thread
From: Honnappa Nagarahalli @ 2019-05-03 15:02 UTC (permalink / raw)
  To: yskoh, jerinj, thomas
  Cc: dev, bruce.richardson, pbhagavatula, shahafs,
	Gavin Hu (Arm Technology China),
	stable, Luca Boccassi, Honnappa Nagarahalli, nd, nd

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension
  2019-05-02  1:58 [dpdk-dev] [PATCH 1/2] build: add option for " Yongseok Koh
@ 2019-05-03 12:28 ` Yongseok Koh
  2019-05-03 12:28   ` Yongseok Koh
                     ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Yongseok Koh @ 2019-05-03 12:28 UTC (permalink / raw)
  To: jerinj, thomas
  Cc: dev, bruce.richardson, pbhagavatula, shahafs, gavin.hu,
	Honnappa.Nagarahalli, stable

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension
  2019-05-03 12:28 ` [dpdk-dev] [PATCH v2] build: disable " Yongseok Koh
@ 2019-05-03 12:28   ` Yongseok Koh
  2019-05-03 15:02   ` Honnappa Nagarahalli
  2019-05-03 22:13   ` Dharmik Thakkar
  2 siblings, 0 replies; 14+ messages in thread
From: Yongseok Koh @ 2019-05-03 12:28 UTC (permalink / raw)
  To: jerinj, thomas
  Cc: dev, bruce.richardson, pbhagavatula, shahafs, gavin.hu,
	Honnappa.Nagarahalli, stable

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-05-06 21:41 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-03 16:06 [dpdk-dev] [PATCH v2] build: disable armv8 crypto extension Jerin Jacob Kollanukkaran
2019-05-03 16:06 ` Jerin Jacob Kollanukkaran
  -- strict thread matches above, loose matches on Subject: below --
2019-05-02  1:58 [dpdk-dev] [PATCH 1/2] build: add option for " Yongseok Koh
2019-05-03 12:28 ` [dpdk-dev] [PATCH v2] build: disable " Yongseok Koh
2019-05-03 12:28   ` Yongseok Koh
2019-05-03 15:02   ` Honnappa Nagarahalli
2019-05-03 15:02     ` Honnappa Nagarahalli
2019-05-03 16:10     ` Honnappa Nagarahalli
2019-05-03 16:10       ` Honnappa Nagarahalli
2019-05-03 17:50       ` Yongseok Koh
2019-05-03 17:50         ` Yongseok Koh
2019-05-03 22:13   ` Dharmik Thakkar
2019-05-03 22:13     ` Dharmik Thakkar
2019-05-06 21:41     ` Yongseok Koh
2019-05-06 21:41       ` Yongseok Koh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).