Add support for building ossl(4) on powerpc64* by implementing ossl_cpuid and
other support functions for powerpc. The required assembly files for ppc were
already present in-tree.
Details
- Reviewers
jhibbits jhb - Group Reviewers
PowerPC - Commits
- rG3465f14dac7d: ossl: Add support for powerpc64/powerpc64le
The changes were tested using the in-tree tools/tools/crypto/cryptocheck.c tool on both powerpc64 and powerpc64le on a POWER9 system.
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
sys/crypto/openssl/ossl_ppc.c | ||
---|---|---|
8 | Are you copying/modifying a file that already has a Foundation's copyright? If no, you can just have your own copyright header. |
sys/crypto/openssl/ossl_ppc.c | ||
---|---|---|
8 | No, this file (other than the trivial wrapper functions from OpenSSL noted below) was written from scratch. Would it be acceptable to just replace the "Copyright (c) 2023 The FreeBSD Foundation)" text with my own copyright and retain the rest of the comment/license text? |
sys/crypto/openssl/ossl_ppc.c | ||
---|---|---|
8 | Yes, and that's how it supposes to be done. :-) As you are the original author, you should own the copyright, and release it under the license the project accepts. The current form is when you're doing this work under the sponsorship of the Foundation, then the copyright owner would be the Foundation, and note that you write it in the comment. | |
31 | The new code doesn't need $FreeBSD$ anymore. |
sys/crypto/openssl/ossl_ppc.c | ||
---|---|---|
8 |
Thanks for clarifying that -- I'll correct this. | |
31 | Got it, will drop. |
Hi all,
Just a quick ping to see if anybody has the time to review this patch -- it would be greatly appreciated.
sys/crypto/openssl/ossl_ppc.c | ||
---|---|---|
57 | Are you sure it supports MFTB? I thought the mftb instruction was deprecated long ago. PowerISA 3.1 reference states on page 1094 "New programs should use mfspr instead of mftb to access the Time Base." Or does this bit do something else? |
sys/crypto/openssl/ossl_ppc.c | ||
---|---|---|
57 | Good point. I originally referenced the logic in openssl-1.1.1v's ppccap.c which seems to prefer MFTB whenever it is available (i.e. it doesn't cause a SIGILL during the feature probe) as well as FreeBSD's mftb() function[1] which always uses the mftb instruction on ppc64 and mfspr on ppc32. On closer inspection though, I don't think any of the code present in the ossl driver here actually checks or uses this flag in any way. If you'd like me to drop the detection and setting of this flag it'd be safe to do so -- care would just need to be taken if any code that depended on it was imported from upstream openssl. [1] https://github.com/freebsd/freebsd-src/blob/main/sys/powerpc/include/cpufunc.h#L137 |
Sorry to ping again, but now that the fix for the kernel_fpu facilities has been merged could I request further review of this patch?
Do you need to update sys/modules/Makefile to enable ossl on powepc64*?
sys/crypto/openssl/ossl_aes.c | ||
---|---|---|
42 | Hmm, arch(7) lists __powerpc__ as the normal macro to use rather than __PPC__, also presumably this should be __powerpc64__ since this review doesn't include 32-bit PPC support? |
Strangely enough I did not need to change sys/modules/Makefile to get the module built and running on powerpc64{,le}, though it seems like for completeness' sake this ought to be done anyways. Will do.
sys/crypto/openssl/ossl_aes.c | ||
---|---|---|
42 | Good observation, will update to powerpc64. |
sys/powerpc/conf/GENERIC64 | ||
---|---|---|
194 | Should you add ossl to GENERIC64LE as well? |
sys/powerpc/conf/GENERIC64 | ||
---|---|---|
194 | Good catch, this was an oversight in my most recent version of the diff. Fixed. |
Hi all,
As far as I can tell, this change still hasn't been committed despite receiving approval. If someone with the appropriate privileges could do so I'd greatly appreciate it.
Thanks,
Shawn