Page MenuHomeFreeBSD

armv8crypto: add AES-XTS support
ClosedPublic

Authored by val_packett.cool on Jul 21 2019, 7:19 PM.
Tags
Referenced Files
F105145468: D21017.diff
Thu, Dec 12, 8:30 PM
Unknown Object (File)
Sat, Dec 7, 8:40 PM
Unknown Object (File)
Thu, Nov 21, 9:01 PM
Unknown Object (File)
Tue, Nov 19, 3:42 PM
Unknown Object (File)
Tue, Nov 19, 3:28 PM
Unknown Object (File)
Tue, Nov 19, 1:41 PM
Unknown Object (File)
Sun, Nov 17, 12:16 AM
Unknown Object (File)
Thu, Nov 14, 3:10 PM

Details

Summary

Straightforward(ish) port from aesni, without unrolling (block8).

Quick speed test: dd if=/dev/zero of=/dev/md0.eli bs=1m reports ~385 MB/s instead of ~60 MB/s on a Cortex-A72 @ 2GHz (Marvell Armada8k).

Test Plan

Tested with opencrypto tests. (D21018 enables them on aarch64)

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

manu added inline comments.
sys/crypto/armv8/armv8_crypto.c
188

Why did you change the argument here ?
You have the key and the key length in the struct cryptoini

sys/crypto/armv8/armv8_crypto.c
188

Look at the calls — in the CRD_F_KEY_EXPLICIT case, instead of cryptoini, we have cryptodesc. This is how aesni deals with this.

val_packett.cool edited the summary of this revision. (Show Details)

Rebased on top of the crypto rework that just landed (D23677). Implemented crp->crp_cipher_key handling.

I'm sorry that this is taking a while to get reviewed. I'd like to help get it in. Would be willing to rebase the patch and re-upload? It doesn't apply for me.

Looks fine. You should look at unrolling the loop to 3 or 4 rounds. Looking at the A72 optimization guide, it shows that there is a 3 cycle latency, but throughput of 1. Section 4.10 gives example showing three pairs to achieve max perf.

A72 opt guide:
https://developer.arm.com/documentation/uan0016/a

It would also be good to test with 'cryptocheck -a all -d armv8crypto0 -z' though the NIST KAT are probably sufficient coverage already for XTS.

Hi,

I was able to rebase this patch with some very minor tweaks, and verify the results using both cryptocheck and cryptotest.py.

Are there any remaining concerns with the patch? I would like to see this committed in the near future.

I haven't reviewed the actual XTS bits in armv8_crypto_wrap.c, but the rest all looks good to me.

This revision was not accepted when it landed; it landed in state Needs Review.Jan 7 2021, 7:37 PM
This revision was automatically updated to reflect the committed changes.