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

[Yongseok Koh <yskoh@mellanox.com>]

On Apr 15, 2019, at 1:13 PM, Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:

>>>> 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.
>> 
> I do not know the answer, will have to check with the compiler team. I will get back on this.

Any update yet?

Thanks 
Yongseok