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

Support OpenBSD on arm64 #962

Merged
merged 9 commits into from
May 8, 2023
Merged

Support OpenBSD on arm64 #962

merged 9 commits into from
May 8, 2023

Conversation

knightjoel
Copy link
Contributor

Issues:

n/a

Description of changes:

This PR basically provides OPENSSL_cpuid_setup() for OpenBSD/aarch64.

Call-outs:

The cpu_aarch64_openbsd.c file is lifted from OpenBSD's patch set to Chromium.

Testing:

A standalone build of aws-lc now succeeds as does building aws-lc as part of the Python CRT bindings. Tested on OpenBSD 7.3.

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

@skmcgrail skmcgrail requested review from skmcgrail and dkostic April 18, 2023 17:53
extern uint32_t OPENSSL_armcap_P;

void OPENSSL_cpuid_setup(void) {
int isar0_mib[] = { CTL_MACHDEP, CPU_ID_AA64ISAR0 };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we please add a reference where all these macros come from?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can, but I'm not sure the extent of what you're asking. Is it just for the macros on line 32? I don't see references in any of the other cpu_*.c files for macros they use.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a comment that they are defined in "X" header file is sufficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it look like what you're expecting now?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. I just changed the comment style and updated the PR with the main branch.

@dkostic dkostic enabled auto-merge (squash) April 28, 2023 21:08
@dkostic dkostic merged commit c489027 into aws:main May 8, 2023
WillChilds-Klein pushed a commit to WillChilds-Klein/aws-lc that referenced this pull request May 8, 2023
justsmth added a commit that referenced this pull request Dec 5, 2024
This reverts commit d36bf85.

### Issues:
Resolves AWS-LC-496

### Description of changes: 
d36bf85 was a merge from upstream. BoringSSL removed some 32-bit ARM
stuff for OpenBSD, but the merge to aws-lc removed some aarch64 stuff,
breaking the build on OpenBSD/arm64 platform (a regression from #962).

### Testing:

I can successfully build aws-lc on OpenBSD/arm64 with this PR.

```
~/aws-lc-obsd-aarch64% cmake --build ./build
[...]
[588/595] Linking CXX executable ssl/test/bssl_shim
ld: warning: ssl_transfer.cc:30 (/home/jwk/aws-lc-obsd-aarch64/ssl/test/ssl_transfer.cc:30)(ssl/test/CMakeFiles/bssl_shim.dir/ssl_transfer.cc.o:(SSLTransfer::ResetSSL(TestConfig const*, std::__1::unique_ptr<ssl_st, bssl::internal::Deleter>*))): warning: rand() may return deterministic values, is that what you want?
[591/595] Linking CXX executable crypto/crypto_testld: warning: bio_md_test.cc:185 (/home/jwk/aws-lc-obsd-aarch64/crypto/pkcs7/bio/bio_md_test.cc:185)(crypto/CMakeFiles/crypto_test.dir/pkcs7/bio/bio_md_test.cc.o:(BIOMessageDigestTest_Randomized_Test::TestBody())): warning: rand() may return deterministic values, is that what you want?
[595/595] Linking CXX executable ssl/ssl_test

~/aws-lc-obsd-aarch64% uname -a
OpenBSD arm64 7.6 GENERIC.MP#196 arm64
```


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.

---------

Co-authored-by: Justin Smith <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants