crypto: arm64/sha2: integrate OpenSSL implementations of SHA256/SHA512
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH v3 1/1 crypto: arm64/sha2: integrate OpenSSL implementations of SHA256/SHA512
Hi Ard,
Quoted 25 lines · Ard Biesheuvel
This integrates both the accelerated scalar and the NEON implementations of SHA-224/256 as well as SHA-384/512 from the OpenSSL project.
Relative performance compared to the respective generic C versions:
| SHA256-scalar | SHA256-NEON* | SHA512 | ------------+-----------------+--------------+----------+ Cortex-A53 | 1.63x | 1.63x | 2.34x | Cortex-A57 | 1.43x | 1.59x | 1.95x | Cortex-A73 | 1.26x | 1.56x | ? |The core crypto code was authored by Andy Polyakov of the OpenSSL project, in collaboration with whom the upstream code was adapted so that this module can be built from the same version of sha512-armv8.pl.
The version in this patch was taken from OpenSSL commit
866e505e0d66 sha/asm/sha512-armv8.pl: add NEON version of SHA256.* The core SHA algorithm is fundamentally sequential, but there is a secondary transformation involved, called the schedule update, which can be performed independently. The NEON version of SHA-224/SHA-256 only implements this part of the algorithm using NEON instructions, the sequential part is always done using scalar instructions.
Quoted 8 lines
Signed-off-by Ard Biesheuvel <[email protected]>---
v3: at Will's request, the generated assembly files are now included
as .S_shipped files, for which generic build rules are defined already. Note that this has caused issues in the past with patchwork, so for Herbert's convenience, the patch can be pulled from http://git.kernel.org/cgit/linux/kernel/git/ardb/linux.git,
branch arm64-sha256 (based on today's cryptodev)
Thanks.
Looking at the generated code, I see references to __ARMEB__ and __ILP32__. The former is probably a bug, whilst the second is not required. There are also some commented out instructions, which is weird.
Will
A message that is not in the archive (sent off-list or not yet received): CAKv+Gu_Y-ik0L9mh4z-g3fHKjPt780_znCQNU-jAwXifLgAo4A@mail.gmail.com
PATCH v3 1/1 crypto: arm64/sha2: integrate OpenSSL implementations of SHA256/SHA512
Quoted 7 lines
(+ Andy)
...
Quoted 2 lines
Looking at the generated code, I see references to
__ARMEB__and
__ILP32__.Quoted 1 line
The former is probably a bug,
Oh! You mean that it should be __AARCH64EB__/__AARCH64EL__! Apparently I simply went on assuming that it would be __ARMEB__/__ARMEL__ even in 64-bit case. As Ard mentioned, there is a number of modules that can be compiled for either 32- or 64-bit case, and supposedly that's where this blunder stems from. Will fix... As for it being actually tested. No, I don't have big-endian AArch64 setup, and big-endian segments in AArch64 are based on experience with endian neutrality elsewhere. I.e. it's based on observations what it takes to achieve the neutrality elsewhere, e.g. ARM, MIPS, PPC, and then exercising equivalent approach even here. Modulo fact that I apparently got macros wrong :-(
Quoted 1 line
Quoted 1 line
whilst the second is not required.
Note that references to __ILP32__ are also inside #ifndef __KERNEL__/#endif, i.e. they won't be even evaluated when compiled for kernel. Or in other words it's shared code artefact just like #if[n]def __KERNEL__ itself. You either disregard it or remove altogether, but then code becomes specific and not shared :-)
Quoted 2 lines
Quoted 2 lines
There are
also some commented out instructions, which is weird.
Commented instructions denote those that are moved into next round in order to improve instruction scheduling on either of processors. In other words it means that you should spot it further below [in generated code]. Yes, it's weird, but it helps me to remember which instructions are moved. On side note it's not uncommon that instruction scheduling is result of compromise. In general I attempt to schedule instruction for near-optimal performance on *multiple* processors, but sometimes you have to make compromises. One is mentioned in commentary section for this very module, the Apple A7 vs. Cortex-A53 thing. Well, it's not like the commented instruction you're observing is the only result of the compromise, it's merely an example of how said compromises can manifest themselves, as somewhat weird irregularities in the code, possibly with commented instructions.
PATCH v3 1/1 crypto: arm64/sha2: integrate OpenSSL implementations of SHA256/SHA512
Quoted 9 lines · Andy Polyakov
Quoted 7 lines
(+ Andy)
...
Quoted 2 lines
Looking at the generated code, I see references to
__ARMEB__and
__ILP32__.Quoted 1 line
The former is probably a bug,
Oh! You mean that it should be __AARCH64EB__/__AARCH64EL__!
Indeed:
$ aarch64-linux-gnu-gcc -dM -E - <<<"" |grep AARCH #define __AARCH64_CMODEL_SMALL__ 1 #define __AARCH64EL__ 1
$ aarch64-linux-gnu-gcc -dM -E -mbig-endian - <<<"" |grep AARCH #define __AARCH64_CMODEL_SMALL__ 1 #define __AARCH64EB__ 1
Signature · 1 line
Ard.
PATCH v3 1/1 crypto: arm64/sha2: integrate OpenSSL implementations of SHA256/SHA512
Quoted 12 lines
Quoted 9 lines
Quoted 7 lines
(+ Andy)
...
Quoted 2 lines
Looking at the generated code, I see references to
__ARMEB__and
__ILP32__.Quoted 1 line
The former is probably a bug,
Oh! You mean that it should be __AARCH64EB__/__AARCH64EL__!
Indeed:
Quoted 7 lines
$ aarch64-linux-gnu-gcc -dM -E - <<<"" |grep AARCH #define __AARCH64_CMODEL_SMALL__ 1 #define __AARCH64EL__ 1$ aarch64-linux-gnu-gcc -dM -E -mbig-endian - <<<"" |grep AARCH #define __AARCH64_CMODEL_SMALL__ 1 #define __AARCH64EB__ 1
As it turns out it wasn't really an overlook, at least not in OpenSSL context, as I force __ARMEB__/__ARMEL__ in 64-bit build through arm_arch.h, which in turn is shared between 32- and 64-bit builds. But shared code should be preferred to be self-sufficient. It doesn't mean that it won't be fixed, only that it wasn't really a bug in OpenSSL context.
PATCH v3 1/1 crypto: arm64/sha2: integrate OpenSSL implementations of SHA256/SHA512
Quoted 16 lines
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Looking at the generated code, I see references to
__ARMEB__and
__ILP32__.Quoted 1 line
The former is probably a bug,
Oh! You mean that it should be __AARCH64EB__/__AARCH64EL__!
Indeed:
Quoted 7 lines
$ aarch64-linux-gnu-gcc -dM -E - <<<"" |grep AARCH #define __AARCH64_CMODEL_SMALL__ 1 #define __AARCH64EL__ 1$ aarch64-linux-gnu-gcc -dM -E -mbig-endian - <<<"" |grep AARCH #define __AARCH64_CMODEL_SMALL__ 1 #define __AARCH64EB__ 1
Quoted 6 lines
As it turns out it wasn't really an overlook, at least not in OpenSSL context, as I force __ARMEB__/__ARMEL__ in 64-bit build through
arm_arch.h, which in turn is shared between 32- and 64-bit builds. But shared code should be preferred to be self-sufficient. It doesn't mean that it won't be fixed, only that it wasn't really a bug in OpenSSL context.