feat: POWER8 hardware AES via vcipher/vcipherlast (ISA 2.07) — 13x speedup - #9932
feat: POWER8 hardware AES via vcipher/vcipherlast (ISA 2.07) — 13x speedup#9932Scottcjn wants to merge 6 commits into
Conversation
Uses ISA 2.07 crypto instructions (vcipher, vcipherlast, vncipher, vncipherlast, vsbox, vpmsumd) instead of scalar T-table approach. 8-way pipeline fills vcipher 7-cycle latency for parallelizable modes. Vectorized counter increment stays in registers (no memory round-trip). Benchmarked on IBM POWER8 S824 (8286-42A): - AES-128-CTR 8-way: 3,595 MiB/s (vs 262 MiB/s T-table = 13.7x) - AES-128-CBC-dec 8-way: 2,796 MiB/s (vs 213 MiB/s = 13.2x) - AES-128-ECB 8-way: 2,931 MiB/s (vs 265 MiB/s = 11.0x) - AES-128-CBC-enc serial: 484 MiB/s (vs 267 MiB/s = 1.8x) All correctness tests pass (CBC + CTR round-trips at 1MB). Co-authored-by: OpenAI GPT-5.4 (vectorized counter increment, 8-way pipeline)
|
Can one of the admins verify this patch? |
Wrap entire file in #if defined(__powerpc64__) so it compiles
cleanly on non-PPC targets (Apple M1, x86, ARM).
Move benchmark main() behind #ifdef POWER8_AES_BENCHMARK.
Add wolfSSL license header.
To build standalone benchmark:
gcc -mcpu=power8 -maltivec -mvsx -O3 -DPOWER8_AES_BENCHMARK \
-o power8_aes_bench ppc64-aes-power8-crypto.c -lrt
|
Hi @Scottcjn, We would be thrilled to have these code changes but need a contributor agreement. Thanks, |
|
Hi Sean — CLA was submitted via support on March 9. Please let me know if you need anything else on that front. Happy to address any technical feedback on the implementation whenever you're ready. The 8-way pipeline approach gives us 3,595 MiB/s on AES-128-CTR which is 13-20x over the T-table path in #9852. |
|
Okay to test. Contributor agreement in review ZD 21321 |
|
Hi @Scottcjn your contributor agreement has been approved. @SparkiDev how would you like to incorporate these changes? Tracking in ZD 21321 |
|
@Scottcjn please add these four macros to .wolfssl_known_macro_extras in the repo root |
|
Hi @SparkiDev — good news, Kareem confirmed the CLA has been approved. Ready to proceed with review whenever you are. The benchmarks on real POWER8 hardware:
13-20x faster than the T-table approach in PR #9852. Let me know if anything needs adjustment. |
|
Pushed two fixes:
The |
|
@dgarske @SparkiDev Great news on the CLA! Happy to help with integration however works best — whether that's rebasing onto a specific branch, splitting the PR, adjusting the configure/cmake detection logic, or anything else. The macro extras and Let me know what you need from my side. |
|
Hi @Scottcjn , sorry for the delay on replying. Sean @SparkiDev is still out on vacation and he'll need to provide feedback before we can finalize this PR. |
|
Hi @dgarske @SparkiDev — gentle follow-up. It has been ~3 weeks since the last update; hope @SparkiDev had a good vacation. Whenever you have a moment to review, the POWER8 AES implementation is still tested and ready (13.5x speedup AES-256-CBC, 20x CTR on real S824 hardware). Happy to rebase, split, or adjust anything that helps integration — just let me know. The PR is |
Hi @Scottcjn , sorry for the delay. @SparkiDev is the right person to work on this. He will respond soon. Thanks |
|
Following up after too long a silence, and with a correction that matters. I went back and tested this code against NIST known-answer vectors instead of the encrypt/decrypt round-trips in the PR description. It fails all of them on ppc64le. The round-trips passed because they only show the code is self-consistent, not that it computes AES. Reproduced by cross-compiling the PR's file for ppc64le and running under qemu-ppc64le with Two separate defects.
The data path has the matching problem, using
wolfSSL passes user buffers into the AES API, so 16 byte alignment cannot be assumed. Separately, None of this showed up in CI because the files are only listed in So the benchmark numbers are measuring something that is not AES on ppc64le, and I should not have presented them the way I did. Please do not merge this as it stands. I would rather close this than leave a broken crypto patch sitting in front of you. The vcipher approach is still right for POWER8, and I am happy to come back with a version that is actually built by One more retraction: the GMAC failure I mentioned against #9852 in the description was never substantiated, and I would not rely on it without a re-test. Thanks for the help with the CLA, and sorry for the noise. |
Summary
This PR adds POWER8 hardware-accelerated AES using the ISA 2.07 vector crypto instructions (
vcipher,vcipherlast,vncipher,vncipherlast,vsbox,vpmsumd). These instructions have been available since POWER8 (2013) and provide single-cycle AES round operations.The intent is to replace the scalar T-table approach in PR #9852 with the hardware crypto unit. That intent is sound. This particular implementation does not currently achieve it correctly — see "Current state" below.
Approach
vcipher/vcipherlast— single-cycle AES round in the vector crypto unitvcipherlatencyCurrent state — known defects
Tested against NIST known-answer vectors (FIPS-197 App B; SP 800-38A F.1.1, F.2.1, F.2.2, F.5.1). All fail on ppc64le. The encrypt/decrypt round-trips that were previously cited as a correctness check only demonstrate that the code is self-consistent, not that it computes AES.
Wrong on little-endian.
AES_set_encrypt_keyassembles each round key word big-endian then stores it as a nativeuint32_t, so on ppc64le every word lands byte-reversed. The data path has the matching problem, usingvec_ld/vec_stwhere an endian-aware load is needed.Alignment.
vec_ld/vec_stdiscard the low 4 bits of the address, so unaligned user buffers produce wrong output. wolfSSL passes user buffers into the AES API, so 16-byte alignment cannot be assumed.CTR counter.
AES_CTR_encrypt_8wayincrements withvec_addon a 4x32 vector — four independent 32-bit adds with no carry propagation, which is not the 128-bit incrementwc_AesCtrEncryptperforms.None of this was caught by CI because the files are only listed in
EXTRA_DIST: they ship in the tarball and are never compiled, so the green PPC64/PPC run did not build any of this code.How the above was tested
Cross-compiled for ppc64le and run under
qemu-ppc64le -cpu power9(qemu's power8 model does not enable the crypto facility here). This is emulation, not silicon — the S824 is not powered up at present. Big-endian ppc64 was not tested; I have no BE sysroot available, and I expect the endianness defect does not affect BE.What is not verified
configure.ac,aes.cdispatch, CPUID detection).vpmsumdis not implemented.Path forward
I would rather fix this properly than leave a broken crypto patch in review. A follow-up would be actually built by
configure.acand dispatched fromaes.c, usewc_-prefixed symbols rather than colliding with the OpenSSL compatAES_set_encrypt_key, handle endianness and alignment, and carry KAT coverage before any performance claim.cc @SparkiDev @dgarske — apologies for the noise, and thanks for the CLA help.