Skip to content
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

ppc64le: support OPENSSL_ppccap ENV variable #1569

Merged
merged 4 commits into from
May 15, 2024

Conversation

justsmth
Copy link
Contributor

@justsmth justsmth commented May 1, 2024

Issues:

CryptoAlg-2425

Description of changes:

  • Architectures for which we have custom assembly implementations need to provide a mechanism for the environment to alter the CPU capabilities that are detected at runtime.
  • This PR adds support for setting a OPENSSL_ppccap environment variable for this purpose.

Testing:

  • Without setting the environment variable: 55ms
[ RUN      ] AESTest.TestVectors
[       OK ] AESTest.TestVectors (55 ms)
  • After setting export OPENSSL_ppccap=0x0000000000000000: 216ms
[ RUN      ] AESTest.TestVectors
[       OK ] AESTest.TestVectors (216 ms)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@justsmth justsmth requested a review from a team as a code owner May 1, 2024 18:11
@jvdsn
Copy link

jvdsn commented May 6, 2024

I can't really test it because I don't have a ppc platform, but the code looks like what I expected.

@skmcgrail
Copy link
Member

skmcgrail commented May 8, 2024

Does this need any any special handling in the delocater? I believe OPENSSL_armcap_P and OPENSSL_ia32cap_P both have special delocate logic handling. Not sure if FIPS is in play for this architecture in question.

@justsmth
Copy link
Contributor Author

justsmth commented May 9, 2024

Does this need any any special handling in the delocater? I believe OPENSSL_armcap_P and OPENSSL_ia32cap_P both have special delocate logic handling. Not sure if FIPS is in play for this architecture in question.

Since this change is only updating the implementation of OPENSSL_cpuid_setup so it can be affected by an environment variable, I don't anticipate it having any impact on the delocater. There are no changes to where/how OPENSSL_ppc64le_hwcap2 is stored.

@codecov-commenter
Copy link

codecov-commenter commented May 9, 2024

Codecov Report

Attention: Patch coverage is 87.50000% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 77.91%. Comparing base (7ef93cb) to head (18015bf).

Files Patch % Lines
crypto/fipsmodule/cpucap/cpu_intel.c 87.50% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1569   +/-   ##
=======================================
  Coverage   77.91%   77.91%           
=======================================
  Files         560      560           
  Lines       94629    94638    +9     
  Branches    13620    13621    +1     
=======================================
+ Hits        73729    73740   +11     
+ Misses      20305    20304    -1     
+ Partials      595      594    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@justsmth justsmth merged commit e89e569 into aws:main May 15, 2024
81 checks passed
@justsmth justsmth deleted the ppc64le-no-asm branch May 15, 2024 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants