| Message ID | 1497611869-6126-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| Headers | show |
| Series | crypto: aes - allow generic AES to be omitted | expand |
Hi Ard, On Fri, Jun 16, 2017 at 01:17:43PM +0200, Ard Biesheuvel wrote: > The generic AES driver uses 16 lookup tables of 1 KB each, and has > encryption and decryption routines that are fully unrolled. Given how > the dependencies between this code and other drivers are declared in > Kconfig files, this code is always pulled into the core kernel, even > if it is usually superseded at runtime by accelerated drivers that > exist for many architectures. > > This leaves us with 25 KB of dead code in the kernel, which is negligible > in typical environments, but which is actually a big deal for the IoT > domain, where every kilobyte counts. > > Also, the scalar, table based AES routines that exist for ARM, arm64, i586 > and x86_64 share the lookup tables with AES generic, and may be invoked > occasionally when the time-invariant AES-NI or other special instruction > drivers are called in interrupt context, at which time the SIMD register > file cannot be used. Pulling 16 KB of code and 9 KB of instructions into > the L1s (and evicting what was already there) when a softirq happens to > be handled in the context of an interrupt taken from kernel mode (which > means no SIMD on x86) is also something that we may like to avoid, by > falling back to a much smaller and moderately less performant driver. > (Note that arm64 will be updated shortly to supply fallbacks for all > SIMD based AES implementations, which will be based on the core routines > [if they are accepted].) > > For the reasons above, this series refactors the way the various AES > implementations are wired up, to allow the generic version in > crypto/aes_generic.c to be omitted from the build entirely. > This looks better now. I think the help text and prompts could still use some improvement. For the prompts, on x86_64 now I see: -*- AES cipher algorithms [*] Fixed time AES cipher [*] AES cipher algorithms (x86_64) [*] AES cipher algorithms (AES-NI) The first is actually the generic table-based implementation now, and it can be deselected if the generic fixed-time implementation is selected and the x86_64 table-based implementation is deselected. How about making the prompts be: AES cipher algorithm (generic, table-based) AES cipher algorithm (generic, time-invariant) AES cipher algorithm (x86_64, table-based) AES cipher algorithm (AES-NI) For the help text, removing the Wikipedia-style boilerplate is good, but IMO the help text should at least spell out "AES (Advanced Encryption Standard)". It's "obvious" to people familiar with crypto algorithms, but I always find it annoying when Kconfig options elsewhere in the kernel use unfamiliar acronyms which the developers didn't bother to spell out because it was "obvious" to them. The help text could also give a bit more information to help people decide which options to enable. For example, the help for CRYPTO_AES_X86_64 could say that it's only useful on older processors that do not have AES-NI instructions, and that the AES-NI implementation, if enabled, will take priority on newer processors. Similarly for the generic implementations, though note that the user may still be required to enable at least one of them as a fallback. Also, the AES-NI and ARMv8-CE implementations are not only time-invariant but also the fastest --- and therefore strongly recommended to enable. Eric
On 19 June 2017 at 05:15, Eric Biggers <ebiggers3@gmail.com> wrote: > Hi Ard, > > On Fri, Jun 16, 2017 at 01:17:43PM +0200, Ard Biesheuvel wrote: >> The generic AES driver uses 16 lookup tables of 1 KB each, and has >> encryption and decryption routines that are fully unrolled. Given how >> the dependencies between this code and other drivers are declared in >> Kconfig files, this code is always pulled into the core kernel, even >> if it is usually superseded at runtime by accelerated drivers that >> exist for many architectures. >> >> This leaves us with 25 KB of dead code in the kernel, which is negligible >> in typical environments, but which is actually a big deal for the IoT >> domain, where every kilobyte counts. >> >> Also, the scalar, table based AES routines that exist for ARM, arm64, i586 >> and x86_64 share the lookup tables with AES generic, and may be invoked >> occasionally when the time-invariant AES-NI or other special instruction >> drivers are called in interrupt context, at which time the SIMD register >> file cannot be used. Pulling 16 KB of code and 9 KB of instructions into >> the L1s (and evicting what was already there) when a softirq happens to >> be handled in the context of an interrupt taken from kernel mode (which >> means no SIMD on x86) is also something that we may like to avoid, by >> falling back to a much smaller and moderately less performant driver. >> (Note that arm64 will be updated shortly to supply fallbacks for all >> SIMD based AES implementations, which will be based on the core routines >> [if they are accepted].) >> >> For the reasons above, this series refactors the way the various AES >> implementations are wired up, to allow the generic version in >> crypto/aes_generic.c to be omitted from the build entirely. >> > > This looks better now. I think the help text and prompts could still use some > improvement. For the prompts, on x86_64 now I see: > > -*- AES cipher algorithms > [*] Fixed time AES cipher > [*] AES cipher algorithms (x86_64) > [*] AES cipher algorithms (AES-NI) > > The first is actually the generic table-based implementation now, and it can be > deselected if the generic fixed-time implementation is selected and the x86_64 > table-based implementation is deselected. How about making the prompts be: > > AES cipher algorithm (generic, table-based) > AES cipher algorithm (generic, time-invariant) > AES cipher algorithm (x86_64, table-based) > AES cipher algorithm (AES-NI) > > For the help text, removing the Wikipedia-style boilerplate is good, but IMO the > help text should at least spell out "AES (Advanced Encryption Standard)". It's > "obvious" to people familiar with crypto algorithms, but I always find it > annoying when Kconfig options elsewhere in the kernel use unfamiliar acronyms > which the developers didn't bother to spell out because it was "obvious" to > them. > > The help text could also give a bit more information to help people decide which > options to enable. For example, the help for CRYPTO_AES_X86_64 could say that > it's only useful on older processors that do not have AES-NI instructions, and > that the AES-NI implementation, if enabled, will take priority on newer > processors. Similarly for the generic implementations, though note that the > user may still be required to enable at least one of them as a fallback. Also, > the AES-NI and ARMv8-CE implementations are not only time-invariant but also the > fastest --- and therefore strongly recommended to enable. > Thanks Eric, all good feedback. I will incorporate it into the next respin. -- Ard.
The generic AES driver uses 16 lookup tables of 1 KB each, and has encryption and decryption routines that are fully unrolled. Given how the dependencies between this code and other drivers are declared in Kconfig files, this code is always pulled into the core kernel, even if it is usually superseded at runtime by accelerated drivers that exist for many architectures. This leaves us with 25 KB of dead code in the kernel, which is negligible in typical environments, but which is actually a big deal for the IoT domain, where every kilobyte counts. Also, the scalar, table based AES routines that exist for ARM, arm64, i586 and x86_64 share the lookup tables with AES generic, and may be invoked occasionally when the time-invariant AES-NI or other special instruction drivers are called in interrupt context, at which time the SIMD register file cannot be used. Pulling 16 KB of code and 9 KB of instructions into the L1s (and evicting what was already there) when a softirq happens to be handled in the context of an interrupt taken from kernel mode (which means no SIMD on x86) is also something that we may like to avoid, by falling back to a much smaller and moderately less performant driver. (Note that arm64 will be updated shortly to supply fallbacks for all SIMD based AES implementations, which will be based on the core routines [if they are accepted].) For the reasons above, this series refactors the way the various AES implementations are wired up, to allow the generic version in crypto/aes_generic.c to be omitted from the build entirely. Patch #1 removes some bogus 'select CRYPTO_AES' statement. Patch #2 introduces CRYPTO_AES_CORE and its implementation crypto/aes_core.c, which contains the existing key expansion routines, and default encrypt and decrypt routines that are not exposed as a crypto_cipher themselves, but can be pulled in by other AES drivers. These routines only depend on the two 256 byte Sboxes Patch #3 switches the fallback in the AES-NI code to the new, generic encrypt and decrypt routines so it no longer depends on the x86 scalar code or [transitively] on AES-generic. Patch #4 repurposes the CRYPTO_AES Kconfig symbol as an abstract symbol that indicates whether some implementation of AES needs to be available. The existing generic code is now controlled by CRYPTO_AES_GENERIC. Patch #5 updates the Kconfig help text to be more descriptive of what they actually control, rather than duplicating AES's wikipedia entry a number of times. Patch #6 updates the Kconfig logic so CRYPTO_AES_GENERIC can be disabled if any CRYPTO_AES dependencies are satisfied by the fixed time driver. v2: - repurpose CRYPTO_AES and avoid HAVE_AES/NEED_AES Kconfig symbols - don't factor out tables from AES generic to be reused by per arch drivers, since the space saving is moderate (the generic code only), and the drivers weren't made to be small anyway Ard Biesheuvel (6): drivers/crypto/Kconfig: drop bogus CRYPTO_AES dependencies crypto: aes - refactor shared routines into separate core module crypto: x86/aes-ni - switch to generic fallback crypto: aes - repurpose CRYPTO_AES and introduce CRYPTO_AES_GENERIC crypto: aes - add meaningful help text to the various AES drivers crypto: aes - allow generic AES to be replaced by fixed time AES arch/arm/crypto/Kconfig | 8 +- arch/arm64/crypto/Kconfig | 11 +- arch/x86/crypto/aesni-intel_glue.c | 4 +- crypto/Kconfig | 85 ++--- crypto/Makefile | 3 +- crypto/aes_core.c | 333 ++++++++++++++++++++ crypto/aes_generic.c | 178 ----------- crypto/aes_ti.c | 305 ++---------------- drivers/crypto/Kconfig | 13 +- include/crypto/aes.h | 6 + net/sunrpc/Kconfig | 3 +- 11 files changed, 407 insertions(+), 542 deletions(-) create mode 100644 crypto/aes_core.c -- 2.7.4