Skip to content

Split up aarch64's "crypto" feature? #614

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

Closed
alexcrichton opened this issue Dec 10, 2018 · 7 comments
Closed

Split up aarch64's "crypto" feature? #614

alexcrichton opened this issue Dec 10, 2018 · 7 comments

Comments

@alexcrichton
Copy link
Member

First brought up here, but we may not want to follow in LLVM's footsteps for this!

@gnzlbg
Copy link
Contributor

gnzlbg commented Dec 10, 2018

I think this was already discussed somewhere else, e.g., rust-lang/rust#29717 (comment) .

That is, we offer crypto because ARM "specifies" this feature (and GCC and Clang also do so), but there is nothing standing in the way of exposing each of the features that crypto enables independently. What would need to be done is:

  • white list the new features in rust-lang/rust
  • update the #[target_feature] of intrinsics requiring crypto to only require exactly the "sub-features" they need

And that's it, everything should still work "as is" for those enabling crypto.

@valpackett
Copy link
Contributor

hmm, are there any actual chips that only expose some of the crypto features but not others? I've only ever seen AES+PMULL,SHA1,SHA2,CRC32 together..

@makotokato
Copy link
Contributor

hmm, are there any actual chips that only expose some of the crypto features but not others? I've only ever seen AES+PMULL,SHA1,SHA2,CRC32 together..

RasPi3 (and RasPi4?) doesn't support ARM crypto extension, so it only support CRC32.

@newpavlov
Copy link
Contributor

newpavlov commented Jan 6, 2020

The crypto feature looks like a small mess to me. The problem is that it means different features for different ARM versions. See here (click on the "cryptographic extensions" link). For ARMv8.4-A it includes SHA1, SHA256, SHA512, SHA3, AES, SM3, SM4, while for older versions only SHA1, SHA256, AES. Thus I think we should use finer-grained features to avoid confusion. Ideally it would be nice to introduce separate sha1, sha2, sha512 and sha3 target features, which would cover only relevant instructions, instead of using sha3 which can mean different things for different versions.

@briansmith
Copy link

hmm, are there any actual chips that only expose some of the crypto features but not others?

Linux removed aes but not pmull or other crypto features from being reported on getauxval() to work around a bug in some ARM64 processors (in 32-bit mode): torvalds/linux@44b3834.

Newer versions of ARMv8 do (and presumably all versions of ARMv9 will) specify that they are to be split; see https://developer.arm.com/downloads/-/exploration-tools/feature-names-for-a-profile, section "Features introduced prior to 2020," e.g. "Note that some names have been split into two; for example, ARMv8.0-AES is split into AES and PMULL."

Should this be closed now? I see that the macro is working for "pmull", "aes", and "sha2". Is there any more to be done?

@Amanieu
Copy link
Member

Amanieu commented Oct 29, 2022

The crypto feature has already been split on aarch64, but not on arm. However we can still fix this since the arm target features are still unstable.

@Amanieu
Copy link
Member

Amanieu commented May 25, 2023

The crypto feature has now been completely removed.

@Amanieu Amanieu closed this as completed May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants