[v3] build: disable armv8 crypto extension

Message ID 20190507211140.43569-1-yskoh@mellanox.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series [v3] build: disable armv8 crypto extension |

Checks

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

Commit Message

Yongseok Koh May 7, 2019, 9:11 p.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.

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

Comments

Honnappa Nagarahalli May 13, 2019, 7:26 p.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.
> 
> 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>
  
Thomas Monjalon June 3, 2019, 11:11 p.m. UTC | #2
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
  

Patch

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