Fix detection of ARMv7 and ARM64 CPU features on FreeBSD#17082
Closed
allanjude wants to merge 1 commit intoopenssl:masterfrom
Closed
Fix detection of ARMv7 and ARM64 CPU features on FreeBSD#17082allanjude wants to merge 1 commit intoopenssl:masterfrom
allanjude wants to merge 1 commit intoopenssl:masterfrom
Conversation
OpenSSL assumes AT_HWCAP = 16 (as on Linux), but on FreeBSD AT_HWCAP = 25 Switch to using AT_HWCAP, and setting it to 16 if it is not defined. OpenSSL calls elf_auxv_info() with AT_CANARY which returns ENOENT resulting in all ARM acceleration features being disabled. CLA: trivial
Contributor
Author
|
This replaces #17079 |
freebsd-git
pushed a commit
to freebsd/freebsd-ports
that referenced
this pull request
Nov 20, 2021
Upstream: * openssl/openssl#17082 * openssl/openssl#17084 PR: 259937 Submitted by: allanjude Differential Revision: https://reviews.freebsd.org/D33062
t8m
approved these changes
Nov 22, 2021
t8m
added
branch: 1.1.1
Member
|
Good for all branches. I am OK with CLA: trivial, although it is borderline case. |
freebsd-git
pushed a commit
to freebsd/freebsd-src
that referenced
this pull request
Nov 22, 2021
OpenSSL assumes the same value for AT_HWCAP=16 (Linux) So it ends up calling elf_auxv_info() with AT_CANARY which returns ENOENT, and all acceleration features are disabled. With this, my ARM64 test machine runs the benchmark `openssl speed -evp aes-256-gcm` nearly 20x faster going from 100 MB/sec to 2000 MB/sec It also improves sha256 from 300 MB/sec to 1800 MB/sec This fix has been accepted but not yet merged upstream: openssl/openssl#17082 PR: 259937 Reviewed by: manu, imp MFC after: immediate Relnotes: yes Fixes: 88e852c ("OpenSSL: Merge OpenSSL 1.1.1j") Sponsored by: Ampere Computing LLC Sponsored by: Klara Inc. Differential Revision: https://reviews.freebsd.org/D33060
freebsd-git
pushed a commit
to freebsd/freebsd-src
that referenced
this pull request
Nov 22, 2021
OpenSSL assumes the same value for AT_HWCAP=16 (Linux) So it ends up calling elf_auxv_info() with AT_CANARY which returns ENOENT, and all acceleration features are disabled. With this, my ARM64 test machine runs the benchmark `openssl speed -evp aes-256-gcm` nearly 20x faster going from 100 MB/sec to 2000 MB/sec It also improves sha256 from 300 MB/sec to 1800 MB/sec This fix has been accepted but not yet merged upstream: openssl/openssl#17082 PR: 259937 Reviewed by: manu, imp MFC after: immediate Relnotes: yes Fixes: 88e852c ("OpenSSL: Merge OpenSSL 1.1.1j") Sponsored by: Ampere Computing LLC Sponsored by: Klara Inc. Differential Revision: https://reviews.freebsd.org/D33060 (cherry picked from commit d9bb798)
jrtc27
reviewed
Nov 22, 2021
| * ARM puts the feature bits for Crypto Extensions in AT_HWCAP2, whereas | ||
| * AArch64 used AT_HWCAP. | ||
| */ | ||
| # ifndef AT_HWCAP |
Contributor
There was a problem hiding this comment.
Should these not be under #ifdef __linux__? Otherwise you still risk the same thing happening on FreeBSD if something gets screwed up and sys/auxv.h isn't included or the macros not defined for whatever reason.
Contributor
Author
There was a problem hiding this comment.
I don't think linux would be correct. The use is later guarded by OSSL_IMPLEMENT_GETAUXVAL which is only set on __FreeBSD_version >= 1200000, but, it does raise the question about android etc.
Contributor
There was a problem hiding this comment.
I think the current state of the PR is commensurate with what OpenSSL does already in analogous situations. Yes, there may be some edge cases where it breaks, but we are going to have to rely on user reports to find those cases.
Contributor
There was a problem hiding this comment.
I do think #ifdef __linux__ would be appropriate. The AT_* values are OSABI-specific and could vary on other operating systems. Note that on FreeBSD it was merely luck that 16 happened to be AT_CANARY and that AT_CANARY happened to always fail to be read due to its special nature vs returning random bits. A mismatch on another OS might instead result in parsing a user space address or some other field like a user ID as the HWCAP bit mask.
freebsd-git
pushed a commit
to freebsd/freebsd-src
that referenced
this pull request
Nov 22, 2021
OpenSSL assumes the same value for AT_HWCAP=16 (Linux) So it ends up calling elf_auxv_info() with AT_CANARY which returns ENOENT, and all acceleration features are disabled. With this, my ARM64 test machine runs the benchmark `openssl speed -evp aes-256-gcm` nearly 20x faster going from 100 MB/sec to 2000 MB/sec It also improves sha256 from 300 MB/sec to 1800 MB/sec This fix has been accepted but not yet merged upstream: openssl/openssl#17082 PR: 259937 Reviewed by: manu, imp MFC after: immediate Relnotes: yes Fixes: 88e852c ("OpenSSL: Merge OpenSSL 1.1.1j") Sponsored by: Ampere Computing LLC Sponsored by: Klara Inc. Differential Revision: https://reviews.freebsd.org/D33060 (cherry picked from commit d9bb798)
kaduk
approved these changes
Nov 22, 2021
| * ARM puts the feature bits for Crypto Extensions in AT_HWCAP2, whereas | ||
| * AArch64 used AT_HWCAP. | ||
| */ | ||
| # ifndef AT_HWCAP |
Contributor
There was a problem hiding this comment.
I think the current state of the PR is commensurate with what OpenSSL does already in analogous situations. Yes, there may be some edge cases where it breaks, but we are going to have to rely on user reports to find those cases.
kaduk
added
approval: done
Contributor
|
For completeness: I am okay with CLA: trivial here. |
freebsd-git
pushed a commit
to freebsd/freebsd-src
that referenced
this pull request
Nov 22, 2021
OpenSSL assumes the same value for AT_HWCAP=16 (Linux) So it ends up calling elf_auxv_info() with AT_CANARY which returns ENOENT, and all acceleration features are disabled. With this, my ARM64 test machine runs the benchmark `openssl speed -evp aes-256-gcm` nearly 20x faster going from 100 MB/sec to 2000 MB/sec It also improves sha256 from 300 MB/sec to 1800 MB/sec This fix has been accepted but not yet merged upstream: openssl/openssl#17082 PR: 259937 Reviewed by: manu, imp Approved by: re (gjb) Relnotes: yes Fixes: 88e852c ("OpenSSL: Merge OpenSSL 1.1.1j") Sponsored by: Ampere Computing LLC Sponsored by: Klara Inc. Differential Revision: https://reviews.freebsd.org/D33060 (cherry picked from commit d9bb798) (cherry picked from commit 0ed191d)
freebsd-git
pushed a commit
to freebsd/freebsd-ports
that referenced
this pull request
Nov 23, 2021
Upstream: * openssl/openssl#17082 * openssl/openssl#17085 PR: 259937 Submitted by: allanjude Differential Revision: https://reviews.freebsd.org/D33061
Collaborator
|
24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually. |
freebsd-git
pushed a commit
to freebsd/freebsd-src
that referenced
this pull request
Nov 23, 2021
…erpc64le Summary: 1. openssl/openssl@34ab13b needs to be merged for ELFv2 support on big-endian. 2. crypto/openssl/crypto/ppccap.c needs to be patched. Same reason as in openssl/openssl#17082. Approved by: jkim, jhibbits MFC after: 1 month Differential Revision: https://reviews.freebsd.org/D33076
t8m
added
approval: ready to merge
openssl-machine
pushed a commit
that referenced
this pull request
Nov 24, 2021
OpenSSL assumes AT_HWCAP = 16 (as on Linux), but on FreeBSD AT_HWCAP = 25 Switch to using AT_HWCAP, and setting it to 16 if it is not defined. OpenSSL calls elf_auxv_info() with AT_CANARY which returns ENOENT resulting in all ARM acceleration features being disabled. CLA: trivial Reviewed-by: Ben Kaduk <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from #17082)
openssl-machine
pushed a commit
that referenced
this pull request
Nov 24, 2021
OpenSSL assumes AT_HWCAP = 16 (as on Linux), but on FreeBSD AT_HWCAP = 25 Switch to using AT_HWCAP, and setting it to 16 if it is not defined. OpenSSL calls elf_auxv_info() with AT_CANARY which returns ENOENT resulting in all ARM acceleration features being disabled. CLA: trivial Reviewed-by: Ben Kaduk <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from #17082) (cherry picked from commit c1dabe2)
openssl-machine
pushed a commit
that referenced
this pull request
Nov 24, 2021
OpenSSL assumes AT_HWCAP = 16 (as on Linux), but on FreeBSD AT_HWCAP = 25 Switch to using AT_HWCAP, and setting it to 16 if it is not defined. OpenSSL calls elf_auxv_info() with AT_CANARY which returns ENOENT resulting in all ARM acceleration features being disabled. CLA: trivial Reviewed-by: Ben Kaduk <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from #17082) (cherry picked from commit c1dabe2)
Member
|
Merged to master, 3.0 and 1.1.1 branches. Thank you for your contribution. |
ocochard
pushed a commit
to ocochard/freebsd-ports
that referenced
this pull request
Nov 25, 2021
Upstream: * openssl/openssl#17082 * openssl/openssl#17084 PR: 259937 Submitted by: allanjude Differential Revision: https://reviews.freebsd.org/D33062
ocochard
pushed a commit
to ocochard/freebsd-ports
that referenced
this pull request
Nov 25, 2021
Upstream: * openssl/openssl#17082 * openssl/openssl#17085 PR: 259937 Submitted by: allanjude Differential Revision: https://reviews.freebsd.org/D33061
freebsd-git
pushed a commit
to freebsd/freebsd-src
that referenced
this pull request
Dec 29, 2021
…erpc64le Summary: 1. openssl/openssl@34ab13b needs to be merged for ELFv2 support on big-endian. 2. crypto/openssl/crypto/ppccap.c needs to be patched. Same reason as in openssl/openssl#17082. Approved by: jkim, jhibbits, alfredo (MFC to stable/13) MFC after: 1 month Differential Revision: https://reviews.freebsd.org/D33076 (cherry picked from commit 3a60869)
bsdjhb
pushed a commit
to bsdjhb/cheribsd
that referenced
this pull request
Jan 20, 2022
OpenSSL assumes the same value for AT_HWCAP=16 (Linux) So it ends up calling elf_auxv_info() with AT_CANARY which returns ENOENT, and all acceleration features are disabled. With this, my ARM64 test machine runs the benchmark `openssl speed -evp aes-256-gcm` nearly 20x faster going from 100 MB/sec to 2000 MB/sec It also improves sha256 from 300 MB/sec to 1800 MB/sec This fix has been accepted but not yet merged upstream: openssl/openssl#17082 PR: 259937 Reviewed by: manu, imp MFC after: immediate Relnotes: yes Fixes: 88e852c ("OpenSSL: Merge OpenSSL 1.1.1j") Sponsored by: Ampere Computing LLC Sponsored by: Klara Inc. Differential Revision: https://reviews.freebsd.org/D33060
bsdjhb
pushed a commit
to bsdjhb/cheribsd
that referenced
this pull request
Jan 20, 2022
…erpc64le Summary: 1. openssl/openssl@34ab13b needs to be merged for ELFv2 support on big-endian. 2. crypto/openssl/crypto/ppccap.c needs to be patched. Same reason as in openssl/openssl#17082. Approved by: jkim, jhibbits MFC after: 1 month Differential Revision: https://reviews.freebsd.org/D33076
OpenSSL assumes AT_HWCAP = 16 (as on Linux), but on FreeBSD AT_HWCAP = 25
Switch to using AT_HWCAP, and setting it to 16 if it is not defined.
OpenSSL calls elf_auxv_info() with AT_CANARY which returns ENOENT
resulting in all ARM acceleration features being disabled.
CLA: trivial