Arm v8 ISA Crypto extensions #1424

simonbutcher · 2018-03-07T21:12:52Z

ARMv8 Crypto Extensions - as written by @mjosaarinen and originally submitted as #1173. This PR is resubmitted as a branch on the mbedtls repository to allow other members of the Mbed TLS team to push commits to the branch and permit review and rework.

To quote the original PR:

A compact patch that provides AES and GCM implementations that utilize the ARMv8 Crypto Extensions. The config flag is MBEDTLS_ARMV8CE_AES_C, which is disabled by default as we don't do runtime checking for the feature. The new implementation lives in armv8ce_aes.c.

Provides similar functionality to #432
Thanks to Barry O'Rourke and others for that contribtion.

Tested on a Cortex A53 device and QEMU. On a midrange phone the real AES-GCM throughput increases about 4x, while raw AES speed is up to 10x faster.

When cross-compiling, you want to set something like:
export CC='aarch64-linux-gnu-gcc'
export CFLAGS='-Ofast -march=armv8-a+crypto'
scripts/config.pl set MBEDTLS_ARMV8CE_AES_C
QEMU seems to also need
export LDFLAGS='-static'
Then run normal make or cmake etc.

simonbutcher · 2018-03-07T21:14:42Z

Assigning to @gilles-peskine-arm and @yanesca for review.

gilles-peskine-arm

Two major design issues, in addition to a few minor style issues:

Why are we making the primitive functions part of the public interface? If there's no good reason for it, they should remain internal.
If the option is activated at compile time, it gets used at runtime. But on an ARMv8 platform there's a good chance that software is deployed in the form of portable binaries that must support systems both with and without the crypto extensions, so we should allow dynamic choice. It would also make sense to do CE-only builds for (I think rarer) deployments where the CE are guaranteed to be available and code size is at a premium.

Other than that I'm happy with the code. I haven't reviewed the assembly parts for correctness, but if they work (proved by testing), I'm happy with them.

gilles-peskine-arm · 2018-03-07T21:20:07Z

ChangeLog

@@ -25,6 +25,7 @@ Features
     uses PBKDF2-SHA2, such as OpenSSL 1.1. Submitted by Antonio Quartulli,
     OpenVPN Inc. Fixes #1339
   * Add support for public keys encoded in PKCS#1 format. #1122
+   * ARMv8 Crypto Extensions: Faster AES and GCM


Needs improvement. Suggestion:

Support ARMv8 Cryptography Extensions for AES and GCM.

gilles-peskine-arm · 2018-03-07T21:20:31Z

include/mbedtls/armv8ce_aes.h

+ * \file armv8ce_aes.h
+ *
+ * \brief ARMv8 Cryptography Extensions -- Optimized code for AES and GCM
+ *


Cut off the Doxygen part so that the copyright notice is in an ordinary comment.

gilles-peskine-arm · 2018-03-07T21:20:46Z

include/mbedtls/armv8ce_aes.h

+ *
+ * \brief ARMv8 Cryptography Extensions -- Optimized code for AES and GCM
+ *
+ *  Copyright (C) 2006-2017, ARM Limited, All Rights Reserved


gilles-peskine-arm · 2018-03-07T21:23:02Z

include/mbedtls/armv8ce_aes.h

@@ -0,0 +1,60 @@
+/**


Why are these public functions? Why is this not entirely internal to aes.c and gcm.c?

I know that's how it was done for AES-NI, but I don't see a good reason. @mpg Do you know?

Honestly, AES-NI was just doing it the same way as Via Padlock support, so the person who knows the reasons if Paul.

However, if I hazard a guess as to why, I think it's mainly because it looked convenient (avoid clutter in aes.c) and concerns about minimizing the exposed API and ABI were probably given less importance at that time than now.

gilles-peskine-arm · 2018-03-07T21:27:23Z

include/mbedtls/config.h

+ *
+ * Requires: MBEDTLS_HAVE_ASM
+ *
+ * This module utilizes ARMv8 Crypto Extensions for AES and GCM


“utilizes” → “adds support”; “Crypto” → “Cryptography” (let's use the official name); full stop at the end of the sentence.

gilles-peskine-arm · 2018-03-07T21:27:57Z

library/CMakeLists.txt

@@ -6,6 +6,7 @@ set(src_crypto
    aes.c
    aesni.c
    arc4.c
+	armv8ce_aes.c


indent glitch

gilles-peskine-arm · 2018-03-07T21:29:24Z

library/aes.c

@@ -42,7 +42,9 @@
 #if defined(MBEDTLS_AESNI_C)
 #include "mbedtls/aesni.h"
 #endif
-


please keep a blank line

gilles-peskine-arm · 2018-03-07T21:40:33Z

library/aes.c

@@ -851,6 +853,11 @@ int mbedtls_aes_crypt_ecb( mbedtls_aes_context *ctx,
        return( mbedtls_aesni_crypt_ecb( ctx, mode, input, output ) );
 #endif

+#if defined(MBEDTLS_ARMV8CE_AES_C)
+	// We don't do runtime checking for ARMv8 Crypto Extensions


Why not? It could make sense to build without software support, to save code size. But as long as software support is present, I think there should be a fallback.

Since it's impossible to detect the presence of the crypto extensions at runtime in unprivileged mode (unfortunately), this should be a runtime setting — a global variable accessed through a setter function, I guess.

Note that it's possible in principle to have accelerated GCM but not AES or vice versa. I don't know if that happens in practice.

On Linux the setting can be initialized by checking for the pmull and aes flags of /proc/cpuinfo. I don't know if we want to bother implementing that.

You almost certainly don't want to go to /proc/cpuinfo... To detect processor features in runtime, use hwcaps instead:

#include <asm/hwcap.h> #include <sys/auxv.h> int crypto_extensions_present(void) { unsigned long hwcaps = getauxval(AT_HWCAP); return (hwcaps & HWCAP_AES) && (hwcaps & HWCAP_PMULL); }

@pawelmoll Thanks! How portable is this in terms of kernel, libc, toolchain, versions? Can we assume it's always there when compiling for Aarch64/Aarch32 or should we provide a compile-time option?

According to its man page:

The getauxval() function was added to glibc in version 2.16.

(which, it seems, was released in 2012).

As to hwcaps themselves, AArch64 (aka arm64) kernels were providing them from the day one :-) (or at least early enough that you wouldn't care).

As to AArch32/arm environment... It seems that the AES and MULL got into AT_HWCAP2 word:
(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/include/uapi/asm/hwcap.h)
but I must say that I have no crypto-enabled platform I could test it on...

Re AArch32 again: they were added "only" in 5.3
(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm/include/uapi/asm/hwcap.h?id=8258a9895c99cdaacad8edc4748c0a624c710961)

So that's Linux-only, not some portability layer? At this point, for Arch64, we can probably reasonably assume that __linux__ implies ≥5.3. Should we also have a guard on __GLIBC__?

It's definitely a Linux thing, yes. And yes, on AArch64 I'd just assume HW_CAPS have all what you need (checked and the AES and MULL bits were added in 3.13).

As to "non-glibc libc-s", musl and bionic seem to have the function:
http://git.musl-libc.org/cgit/musl/tree/src/misc/getauxval.c
https://android.googlesource.com/platform/bionic/+/master/libc/bionic/getauxval.cpp
but, for example, uclibc doesn't...

Also, there's an old lwn article describing auxv:

Even without using the new library function, an application that wants to access the auxiliary vector merely needs to obtain the address of the location that follows the NULL pointer at the end of the environment list.

gilles-peskine-arm · 2018-03-07T21:42:00Z

library/armv8ce_aes.c

+
+#if defined(MBEDTLS_ARMV8CE_AES_C)
+
+#include <arm_neon.h>


Is this header present in all toolchains for ARMv8 (that we care about)?

It's part of ACLE, I believe, so yes - the intrinsics should be readily available:

https://developer.arm.com/documentation/101028/0006/3--C-language-extensions?lang=en

gilles-peskine-arm · 2018-03-07T21:51:06Z

library/armv8ce_aes.c

+
+    if( mode == MBEDTLS_AES_ENCRYPT )
+    {
+        for( i = ctx->nr - 1; i ; i-- )             // encryption loop


Style: please write i != 0 for the condition. Applies to the decryption loop as well.

gilles-peskine-arm · 2018-03-08T13:31:31Z

library/CMakeLists.txt

@@ -6,6 +6,7 @@ set(src_crypto
    aes.c
    aesni.c
    arc4.c


The new file needs to be added to the VS build as well.

A compact patch that provides AES and GCM implementations that utilize the ARMv8 Crypto Extensions. The config flag is MBEDTLS_ARMV8CE_AES_C, which is disabled by default as we don't do runtime checking for the feature. The new implementation lives in armv8ce_aes.c. Provides similar functionality to #432 Thanks to Barry O'Rourke and others for that contribtion. Tested on a Cortex A53 device and QEMU. On a midrange phone the real AES-GCM throughput increases about 4x, while raw AES speed is up to 10x faster. When cross-compiling, you want to set something like: export CC='aarch64-linux-gnu-gcc' export CFLAGS='-Ofast -march=armv8-a+crypto' scripts/config.pl set MBEDTLS_ARMV8CE_AES_C QEMU seems to also need export LDFLAGS='-static' Then run normal make or cmake etc.

Signed-off-by: Hanno Becker <hanno.becker@arm.com>

simonbutcher requested review from yanesca and gilles-peskine-arm March 7, 2018 21:14

simonbutcher added enhancement mbed TLS team needs-design-approval needs-review Every commit must be reviewed by at least two team members, labels Mar 7, 2018

simonbutcher mentioned this pull request Mar 7, 2018

ARMv8 crypto extensions #1173

Closed

gilles-peskine-arm requested changes Mar 8, 2018

View reviewed changes

gilles-peskine-arm reviewed Mar 8, 2018

View reviewed changes

simonbutcher mentioned this pull request Mar 20, 2018

AARCH64 and Cryptography Extension Support #432

Closed

Patater assigned yanesca Apr 3, 2018

Patater removed the request for review from yanesca April 3, 2018 08:55

Patater added the CLA valid label Apr 3, 2018

simonbutcher added the component-crypto Crypto primitives and low-level interfaces label Jan 9, 2019

bluebike mentioned this pull request May 29, 2019

High CPU usage on Raspberry Pi 3 fluent/fluent-bit#1341

Closed

gilles-peskine-arm added needs-work and removed needs-review Every commit must be reviewed by at least two team members, labels Sep 12, 2019

danh-arm added this to the Backlog milestone May 20, 2020

danh-arm removed the mbed TLS team label May 20, 2020

danh-arm removed this from the Backlog milestone May 20, 2020

wongsyrone mentioned this pull request Aug 17, 2020

[Feature Request] delete trojan-gfw/trojan#508

Closed

hanno-becker mentioned this pull request Aug 18, 2020

Add arm-linux-gnueabi-gcc build #3572

Merged

4 tasks

mjosaarinen added 5 commits September 7, 2020 12:02

ARMv8 Crypto Extensions hooks for AES and GCM

63f2139

ARMv8 Crypto Extensions compile logic

8512301

adapt ChangeLog

c332e2d

MBEDTLS_ARMV8CE_AES_C disabled for full testing in scripts/config.pl

0c2f86d

Hanno Becker added 9 commits September 7, 2020 12:02

Fix style in documentation of MBEDTLS_HAVE_ASM

75bc755

Signed-off-by: Hanno Becker <hanno.becker@arm.com>

Improve ChangeLog entry

be90755

Signed-off-by: Hanno Becker <hanno.becker@arm.com>

Don't include copyright header in Doxygen block in armv8ce_aes.h

74aa040

Signed-off-by: Hanno Becker <hanno.becker@arm.com>

Use official name for 'Armv8 Cryptography Extensions'

bdadc24

Signed-off-by: Hanno Becker <hanno.becker@arm.com>

Fix copy-pasta in documentation for MBEDTLS_ARMV8CE_AES_C

e128636

Signed-off-by: Hanno Becker <hanno.becker@arm.com>

Add empty lines for readability in library/aes.c

e177719

Signed-off-by: Hanno Becker <hanno.becker@arm.com>

Fix coding style in library/armv8ce_aes.c

140fd9c

Signed-off-by: Hanno Becker <hanno.becker@arm.com>

Update Visual Studio project files

15062fa

Signed-off-by: Hanno Becker <hanno.becker@arm.com>

Add documentation on testing Mbed TLS using QEMU syscall emulation

e680037

Signed-off-by: Hanno Becker <hanno.becker@arm.com>

hanno-becker force-pushed the armv8_crypto_extensions branch from 231265b to e680037 Compare September 7, 2020 12:53

1715173329 mentioned this pull request Mar 8, 2021

mbedtls: Implements AES and GCM with ARMv8 Crypto Extensions coolsnowwolf/lede#6491

Merged

daverodgman added the Product Backlog label Mar 30, 2021

daverodgman closed this Jan 5, 2022

daverodgman deleted the armv8_crypto_extensions branch January 5, 2022 17:59

gilles-peskine-arm mentioned this pull request Jan 5, 2022

Support armv8 crypto extensions for AES and GCM #5387

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Arm v8 ISA Crypto extensions #1424

Arm v8 ISA Crypto extensions #1424

simonbutcher commented Mar 7, 2018

simonbutcher commented Mar 7, 2018

gilles-peskine-arm left a comment

gilles-peskine-arm Mar 7, 2018

gilles-peskine-arm Mar 7, 2018

gilles-peskine-arm Mar 7, 2018

gilles-peskine-arm Mar 7, 2018

mpg Mar 12, 2018

gilles-peskine-arm Mar 7, 2018

gilles-peskine-arm Mar 7, 2018

gilles-peskine-arm Mar 7, 2018

gilles-peskine-arm Mar 7, 2018

pawelmoll Jul 10, 2020

gilles-peskine-arm Jul 10, 2020

pawelmoll Jul 10, 2020 •

edited

Loading

pawelmoll Jul 10, 2020

gilles-peskine-arm Jul 10, 2020

pawelmoll Jul 10, 2020

pawelmoll Jul 10, 2020

gilles-peskine-arm Mar 7, 2018

pawelmoll Jul 10, 2020

gilles-peskine-arm Mar 7, 2018

gilles-peskine-arm Mar 8, 2018

@@ @@ -6,6 +6,7 @@ set(src_crypto @@
                   aes.c
                   aesni.c
                   arc4.c

Arm v8 ISA Crypto extensions #1424

Arm v8 ISA Crypto extensions #1424

Conversation

simonbutcher commented Mar 7, 2018

simonbutcher commented Mar 7, 2018

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pawelmoll Jul 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pawelmoll Jul 10, 2020 •

edited

Loading