arm64: enable ossl accelerated AES-GCM on arm64
Needs ReviewPublic
Actions

Authored by gallatin on Mon, Sep 28, 5:22 PM.

Details

Reviewers

markj
andrew
ngie

Summary

Connect the baseline OpenSSL ARMv8 fused AES-GCM kernels to the OCF
ossl driver. Advertise the algorithm only when the system-wide capability set
includes both AES and PMULL.

On arm64, re-run ossl_cpuid via a sysinit() that runs at SI_ORDER_LAST,
since the ossl device attaches before arm64 populates elf_hwcap when
ossl is built into the kernel or pre-loaded at boot.

Prefer OSSL over armv8crypto for AES-GCM sessions while retaining
armv8crypto as a fallback.

This provides about a 50% speedup for a Netflix ktls workload on a small
neoverse N1 board.

Test Plan

Run real Netflix AES-GCM ktls (done)

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

gallatin created this revision.Mon, Sep 28, 5:22 PM

Herald added a reviewer: ngie. · View Herald TranscriptMon, Sep 28, 5:22 PM

Herald added subscribers: emaste, imp. · View Herald Transcript

gallatin requested review of this revision.Mon, Sep 28, 5:22 PM

Run real Netflix AES-GCM ktls (done)

Have you run tools/tools/crypto/cryptocheck.c against this? That would give you more complete coverage.

In D60097#1379088, @markj wrote:

Run real Netflix AES-GCM ktls (done)

Have you run tools/tools/crypto/cryptocheck.c against this? That would give you more complete coverage.

I was not aware of that, so I just ran it in response to your comment. I assume this is passing:

# kldload /d/cryptodev.ko
# ./cryptocheck -v -d ossl -a aes-gcm -z > log
#  grep -Ei 'mismatch|failed|didn.t fail|not supported' log
#

Thank you!

According to the license header in sys/crypto/openssl/ossl_aes_gcm.c, the file came from the OpenSSL project. Have the changes been contributed back yet? If not, please see crypto/openssl/crypto/modes/asm/... for more details. This is where the implementations are for other architectures today: ppc, riscv64, and avx512 (presumably x86*).

FreeBSD/sys/crypto/openssl/ossl.c
4	Please update the copyright.
203–206	Are there any ARMv8 hosts where this wouldn't be true? Would it be worth making this conditional via a kernel tunable, just in case the implementation in armv8crypto is more performant than the ossl one?
506–507	Is there a way to change the startup order to avoid this conflict?
FreeBSD/sys/crypto/openssl/ossl_aarch64.c
4

In D60097#1380343, @ngie wrote:

According to the license header in sys/crypto/openssl/ossl_aes_gcm.c, the file came from the OpenSSL project. Have the changes been contributed back yet? If not, please see crypto/openssl/crypto/modes/asm/... for more details. This is where the implementations are for other architectures today: ppc, riscv64, and avx512 (presumably x86*).

I don't think there is any direct analog in the openssl project. From this history of the file, it entered the tree via @markj's commit 9a3444d91c706dda65040138acbdb8c932213960 (https://reviews.freebsd.org/D39783) as sys/crypto/openssl/amd64/ossl_aes_gcm.c b/sys/crypto/openssl/amd64/ossl_aes_gcm.c as a bridge between openssl apis and OCF apis. It was later moved to sys/crypto/openssl/ossl_aes_gcm.c in 5daf8ed625af70ebb7e4740ab98a6054e9e52329 (https://reviews.freebsd.org/D44274) when ppc64 support was added.

I'm honestly not sure why there is an openssl copyright on this. @markj?

In any case, I don't think there is anything to be contributed back.

In D60097#1380806, @gallatin wrote:

In D60097#1380343, @ngie wrote:

According to the license header in sys/crypto/openssl/ossl_aes_gcm.c, the file came from the OpenSSL project. Have the changes been contributed back yet? If not, please see crypto/openssl/crypto/modes/asm/... for more details. This is where the implementations are for other architectures today: ppc, riscv64, and avx512 (presumably x86*).

I don't think there is any direct analog in the openssl project. From this history of the file, it entered the tree via @markj's commit 9a3444d91c706dda65040138acbdb8c932213960 (https://reviews.freebsd.org/D39783) as sys/crypto/openssl/amd64/ossl_aes_gcm.c b/sys/crypto/openssl/amd64/ossl_aes_gcm.c as a bridge between openssl apis and OCF apis. It was later moved to sys/crypto/openssl/ossl_aes_gcm.c in 5daf8ed625af70ebb7e4740ab98a6054e9e52329 (https://reviews.freebsd.org/D44274) when ppc64 support was added.

I'm honestly not sure why there is an openssl copyright on this. @markj?

Because the code comes from openssl. See the comment at the beginning of the file:

/*
 * This file contains an AES-GCM wrapper implementation from OpenSSL, using
 * AES-NI (x86) or POWER8 Crypto Extensions (ppc). It was ported from
 * cipher_aes_gcm_hw_aesni.inc and it makes use of a generic C implementation
 * for partial blocks, ported from gcm128.c with OPENSSL_SMALL_FOOTPRINT defined.
 */

Indeed, there is nothing to contribute back.

gallatin marked an inline comment as done.Wed, Sep 30, 3:52 PM

gallatin added inline comments.

FreeBSD/sys/crypto/openssl/ossl.c
203–206	No idea. It seems simplest to just not load the module (this is not compiled into GENERIC)
506–507	The other options to deal with this are all worse. move arm64 CPU attachment to happen before devices like other arches: worse because there must be a reason it is the way it is hack the nexus driver to attach after cpus. Worse because it seems dangerous read the registers on THIS cpu at attach and hope that holds true for all CPUs. Much uglier than a sysinit don't support kernel compilation or module pre-load. So this seems like the least bad of a lot of bad options.
FreeBSD/sys/crypto/openssl/ossl_aarch64.c
4	I didn't consider adding 10 lines to a file worthy of adding a copyright, but I see your point given its only 80 lines.

Update copyright as requested by @ngie

ngie mentioned this in D59770: ossl.4: Canonicalize HARDWARE and add powerpc64.Thu, Oct 1, 2:24 AM

jhb added a subscriber: jhb.Thu, Oct 1, 4:21 PM

jhb added inline comments.

FreeBSD/sys/crypto/openssl/ossl.c
203–206	Maybe we can lower the priority of armv8crypto instead in general? I think the only thing armv8crypto handles that ossl doesn't is AES-XTS with your changes here, and I'd like to retire the "bespoke" accelerated software drivers (arvm8crypto and aesni) in favor of ossl(4), so if you've verified that ossl(4) performs at or better than armv8crypto, I'd be happy to instead patch arvm8crypto to use `PROBE_ACCEL_SOFTWARE - 50` or some such in its probesession. (I'd like to do the same for aesni(4) as well at some point.)

gallatin added inline comments.Thu, Oct 1, 4:23 PM

FreeBSD/sys/crypto/openssl/ossl.c
203–206	Yes.. on my test hardware (small N1 setup) ossl gets me ~75Gb/s for my ktls workload, whereas armv8crypto gets 45Gb/s, so its more than 50% better.