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

[Yongseok Koh <yskoh@mellanox.com>]

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

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

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

Copied dpdk-on-arm ML.


Thanks,
Yongseok