-
Notifications
You must be signed in to change notification settings - Fork 638
Adding armv8 crypto extensions to SHA256/224 #1052
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
49bff81
sha256 cyrpto extensions seem to work, want to add neon version if cr…
Martyrshot 8f185b1
Added additional files to CMakeLists.txt
Martyrshot b1f7938
SHA256 armv8 implementation is done. Still need to double check that …
Martyrshot 1c16957
Updated sha2 CMakeList.txt to resolve issue compiling on aws ARM system
Martyrshot 42eac77
I think I have resolved the aws/rock1 build issues
Martyrshot 099e8f5
Removed an resolved TODO comment
Martyrshot 075a802
Fixed a cmake build bug when OQS_DIST_BUILD is "ON"
Martyrshot fcbeaef
Made CMAKE sha2 build more flexible when using OQS_DIST_BUILD
Martyrshot 88b2a70
Signature datasheets (#1053).
xvzcf 39ae75b
Merge branch 'open-quantum-safe:main' into armv8-SHA2
Martyrshot d1d6f96
Fixed a typo in the common CMakeLists.txt file related to SHA2
Martyrshot 89d3141
Merge branch 'open-quantum-safe:main' into armv8-SHA2
Martyrshot a941d12
Add runtime feature detection for macOS on Apple Silicon
dstebila c295251
Changed macos_feature_detection to return unsigned int to fix warning…
Martyrshot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,132 @@ | ||
| // SPDX-License-Identifier: MIT | ||
| #include <stdio.h> | ||
|
|
||
| #include <oqs/common.h> | ||
|
|
||
| #include "sha2.h" | ||
| #include "sha2_local.h" | ||
|
|
||
| #if defined(OQS_DIST_ARM64v8_BUILD) | ||
| #define C_OR_NI(stmt_c, stmt_ni) \ | ||
| do { \ | ||
| if (OQS_CPU_has_extension(OQS_CPU_EXT_ARM_SHA2)) { \ | ||
| stmt_ni; \ | ||
| } else { \ | ||
| stmt_c; \ | ||
| } \ | ||
| } while(0) | ||
| #elif defined(OQS_USE_ARM_SHA2_INSTRUCTIONS) | ||
| #define C_OR_NI(stmt_c, stmt_ni) \ | ||
| stmt_ni | ||
| #else | ||
| #define C_OR_NI(stmt_c, stmt_ni) \ | ||
| stmt_c | ||
| #endif | ||
| void OQS_SHA2_sha224_inc_init(OQS_SHA2_sha224_ctx *state) { | ||
| oqs_sha2_sha224_inc_init_c((sha224ctx *) state); | ||
dstebila marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| void OQS_SHA2_sha224_inc_ctx_clone(OQS_SHA2_sha224_ctx *dest, const OQS_SHA2_sha224_ctx *src) { | ||
| oqs_sha2_sha224_inc_ctx_clone_c((sha224ctx *) dest, (const sha224ctx *) src); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha224_inc_blocks(OQS_SHA2_sha224_ctx *state, const uint8_t *in, size_t inblocks) { | ||
| C_OR_NI( | ||
| oqs_sha2_sha224_inc_blocks_c((sha224ctx *) state, in, inblocks), | ||
| oqs_sha2_sha224_inc_blocks_ni((sha224ctx *) state, in, inblocks) | ||
| ); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha224_inc_finalize(uint8_t *out, OQS_SHA2_sha224_ctx *state, const uint8_t *in, size_t inlen) { | ||
| oqs_sha2_sha224_inc_finalize_c(out, (sha224ctx *) state, in, inlen); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha224_inc_ctx_release(OQS_SHA2_sha224_ctx *state) { | ||
| oqs_sha2_sha224_inc_ctx_release_c((sha224ctx *) state); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha256_inc_init(OQS_SHA2_sha256_ctx *state) { | ||
| oqs_sha2_sha256_inc_init_c((sha256ctx *) state); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha256_inc_ctx_clone(OQS_SHA2_sha256_ctx *dest, const OQS_SHA2_sha256_ctx *src) { | ||
| oqs_sha2_sha256_inc_ctx_clone_c((sha256ctx *) dest, (const sha256ctx *) src); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha256_inc_blocks(OQS_SHA2_sha256_ctx *state, const uint8_t *in, size_t inblocks) { | ||
| C_OR_NI( | ||
| oqs_sha2_sha256_inc_blocks_c((sha256ctx *) state, in, inblocks), | ||
| oqs_sha2_sha256_inc_blocks_ni((sha256ctx *) state, in, inblocks) | ||
| ); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha256_inc_finalize(uint8_t *out, OQS_SHA2_sha256_ctx *state, const uint8_t *in, size_t inlen) { | ||
| oqs_sha2_sha256_inc_finalize_c(out, (sha256ctx *) state, in, inlen); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha256_inc_ctx_release(OQS_SHA2_sha256_ctx *state) { | ||
| oqs_sha2_sha256_inc_ctx_release_c((sha256ctx *) state); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha384_inc_init(OQS_SHA2_sha384_ctx *state) { | ||
| oqs_sha2_sha384_inc_init_c((sha384ctx *)state); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha384_inc_ctx_clone(OQS_SHA2_sha384_ctx *dest, const OQS_SHA2_sha384_ctx *src) { | ||
| oqs_sha2_sha384_inc_ctx_clone_c((sha384ctx *) dest, (const sha384ctx *) src); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha384_inc_blocks(OQS_SHA2_sha384_ctx *state, const uint8_t *in, size_t inblocks) { | ||
| oqs_sha2_sha384_inc_blocks_c((sha384ctx *) state, in, inblocks); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha384_inc_finalize(uint8_t *out, OQS_SHA2_sha384_ctx *state, const uint8_t *in, size_t inlen) { | ||
| oqs_sha2_sha384_inc_finalize_c(out, (sha384ctx *) state, in, inlen); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha384_inc_ctx_release(OQS_SHA2_sha384_ctx *state) { | ||
| oqs_sha2_sha384_inc_ctx_release_c((sha384ctx *) state); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha512_inc_init(OQS_SHA2_sha512_ctx *state) { | ||
| oqs_sha2_sha512_inc_init_c((sha512ctx *)state); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha512_inc_ctx_clone(OQS_SHA2_sha512_ctx *dest, const OQS_SHA2_sha512_ctx *src) { | ||
| oqs_sha2_sha512_inc_ctx_clone_c((sha512ctx *) dest, (const sha512ctx *) src); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha512_inc_blocks(OQS_SHA2_sha512_ctx *state, const uint8_t *in, size_t inblocks) { | ||
| oqs_sha2_sha512_inc_blocks_c((sha512ctx *) state, in, inblocks); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha512_inc_finalize(uint8_t *out, OQS_SHA2_sha512_ctx *state, const uint8_t *in, size_t inlen) { | ||
| oqs_sha2_sha512_inc_finalize_c(out, (sha512ctx *) state, in, inlen); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha512_inc_ctx_release(OQS_SHA2_sha512_ctx *state) { | ||
| oqs_sha2_sha512_inc_ctx_release_c((sha512ctx *) state); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha224(uint8_t *out, const uint8_t *in, size_t inlen) { | ||
| C_OR_NI ( | ||
| oqs_sha2_sha224_c(out, in, inlen), | ||
| oqs_sha2_sha224_ni(out, in, inlen) | ||
| ); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha256(uint8_t *out, const uint8_t *in, size_t inlen) { | ||
| C_OR_NI ( | ||
| oqs_sha2_sha256_c(out, in, inlen), | ||
| oqs_sha2_sha256_ni(out, in, inlen) | ||
| ); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha384(uint8_t *out, const uint8_t *in, size_t inlen) { | ||
| oqs_sha2_sha384_c(out, in, inlen); | ||
| } | ||
|
|
||
| void OQS_SHA2_sha512(uint8_t *out, const uint8_t *in, size_t inlen) { | ||
| oqs_sha2_sha512_c(out, in, inlen); | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually just realizing that this line might be problematic. It's setting
-mcpu=cortex-a53+cryptobut is guarded only byif (OQS_DIST_ARM64v8_BUILD); but there will be certainly many other ARM64v8 CPU's other than Cortex-A53, and even some which don't have crypto extensions (e.g., our Raspberry Pi 3B's which don't versus our ROCK64's which do). Have you tried compiling and running this on other ARM64v8 processors? We have the rasp3b's, the rock64's, as well as an Apple Silicon.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tried it on the roc, raspis and aws instance. I'd be happy to test on apple silicon, but I would need access to a machine since I only have an intel mac. The reason I picked that flag, in particular, is because I have to specify a core in order to "turn on" crypto extensions when compiling on a system that doesn't have them (the
+cryptoportion of-mcpu), anda53is thegenericcore used by liboqs. Without that compile flag the compiler will error saying that the extension isn't supported, but since we do a runtime check whenOQS_DIST_ARM64v8_BUILDis specified, so we need to have those intrinsics compiled into the library.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there is an unrelated compiling issue when using
OQS_DIST_BUILDI get the following error:If I don't use
OQS_DIST_BUILDmy branch builds and all tests pass. (I have tried this on both the liboqs main branch and my branch with the same results). If this is intentional then the above concern isn't an issue, otherwise, I can take a look into finding a solution.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we do want to be able to compile in that configuration, so can you look into it? I think what might be going on there is that the lines 74-92 of common.c are protected by
OQS_DIST_ARM64v8_BUILDwhich in principle would include both Linux ARM (e.g., Raspberry Pi's) and Apple ARM, but the code in that section is solely Linux-focused; for example I think neither<sys/auxv.h>norgetauxvalis available on macOS. This site shows one example of reading the CPU features on macOS. Although for Apple chips it seems like the features we want are always available (at least for now), so maybe we can simplify and just set them automatically without testing?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, I'll see what I can figure out. Would you like me to create a new PR or issue specifically for detecting apple M1 cpu features?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I've done it. It runs for me our M1, but I haven't tested to make sure it doesn't break things on our other ARMs. Can you give a try?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, for sure! Thanks for doing that! I'll try it out right away.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made some small changes and tested on the roc, raspi, was, and apple silicon machine and all tests passed. I also verified that on the apple silicon machine that the sha256 instructions were being used rather than the c implementation. So it should be good to go!