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

[Yongseok Koh <yskoh@mellanox.com>]

> On Apr 29, 2019, at 8:33 PM, Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
> 
>> 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?
> Currently, enabling 'crypto' flag will generate the crypto instructions only when crypto intrinsics are used. However, when 'sha3' (part of 8.2 crypto) flag is enabled, compiler can generate 3-way exclusive OR instructions beyond the intrinsics. Compiler team cannot provide a guarantee that other crypto instructions will not be used beyond the intrinsics.
> 
> The current suggestion is to use GNU indirect function [1] or similar. I am not sure on GNU indirect function portability.
> 
> [1] https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwillnewton.name%2F2013%2F07%2F02%2Fusing-gnu-indirect-functions%2F&amp;data=02%7C01%7Cyskoh%40mellanox.com%7Ce8738c4f725a4ca608ea08d6cd1cac03%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636921920373635167&amp;sdata=kuq6dbpTBfRgokrv2L%2FV4BIM0q1k%2FiL1JaMqCHUc2c0%3D&amp;reserved=0

Thanks for the update,

Then, I think the original patch to have build config is currently okay.
Will submit it again.

thanks
Yongseok