All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: linux-crypto@vger.kernel.org,
	Herbert Xu <herbert@gondor.apana.org.au>,
	David Miller <davem@davemloft.net>,
	Greg KH <gregkh@linuxfoundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Samuel Neves <sneves@dei.uc.pt>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	Arnd Bergmann <arnd@arndb.de>, Eric Biggers <ebiggers@google.com>,
	Andy Lutomirski <luto@kernel.org>, Will Deacon <will@kernel.org>,
	Marc Zyngier <maz@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Martin Willi <martin@strongswan.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>
Subject: Re: [PATCH v2 02/20] crypto: x86/chacha - expose SIMD ChaCha routine as library function
Date: Fri, 4 Oct 2019 15:36:49 +0200	[thread overview]
Message-ID: <20191004133649.GC112631@zx2c4.com> (raw)
In-Reply-To: <20191002141713.31189-3-ard.biesheuvel@linaro.org>

On Wed, Oct 02, 2019 at 04:16:55PM +0200, Ard Biesheuvel wrote:
> Wire the existing x86 SIMD ChaCha code into the new ChaCha library
> interface.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/x86/crypto/chacha_glue.c | 36 ++++++++++++++++++++
>  crypto/Kconfig                |  1 +
>  include/crypto/chacha.h       |  6 ++++
>  3 files changed, 43 insertions(+)
> 
> diff --git a/arch/x86/crypto/chacha_glue.c b/arch/x86/crypto/chacha_glue.c
> index bc62daa8dafd..fd9ef42842cf 100644
> --- a/arch/x86/crypto/chacha_glue.c
> +++ b/arch/x86/crypto/chacha_glue.c
> @@ -123,6 +123,42 @@ static void chacha_dosimd(u32 *state, u8 *dst, const u8 *src,
>  	}
>  }
>  
> +void hchacha_block(const u32 *state, u32 *stream, int nrounds)
> +{
> +	state = PTR_ALIGN(state, CHACHA_STATE_ALIGN);
> +
> +	if (!crypto_simd_usable()) {
> +		hchacha_block_generic(state, stream, nrounds);
> +	} else {
> +		kernel_fpu_begin();
> +		hchacha_block_ssse3(state, stream, nrounds);
> +		kernel_fpu_end();
> +	}
> +}
> +EXPORT_SYMBOL(hchacha_block);

Please correct me if I'm wrong:

The approach here is slightly different from Zinc. In Zinc, I had one
entry point that conditionally called into the architecture-specific
implementation, and I did it inline using #includes so that in some
cases it could be optimized out.

Here, you override the original symbol defined by the generic module
from the architecture-specific implementation, and in there you decide
which way to branch.

Your approach has the advantage that you don't need to #include a .c
file like I did, an ugly yet very effective approach.

But it has two disadvantages:

1. For architecture-specific code that _always_ runs, such as the
  MIPS32r2 implementation of chacha, the compiler no longer has an
  opportunity to remove the generic code entirely from the binary,
  which under Zinc resulted in a smaller module.

2. The inliner can't make optimizations for that call.

Disadvantage (2) might not make much of a difference. Disadvantage (1)
seems like a bigger deal. However, perhaps the linker is smart and can
remove the code and symbol? Or if not, is there a way to make the linker
smart? Or would all this require crazy LTO which isn't going to happen
any time soon?

  parent reply	other threads:[~2019-10-04 13:36 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-02 14:16 [PATCH v2 00/20] crypto: crypto API library interfaces for WireGuard Ard Biesheuvel
2019-10-02 14:16 ` [PATCH v2 01/20] crypto: chacha - move existing library code into lib/crypto Ard Biesheuvel
2019-10-02 14:30   ` Greg KH
2019-10-04 13:21   ` Jason A. Donenfeld
2019-10-02 14:16 ` [PATCH v2 02/20] crypto: x86/chacha - expose SIMD ChaCha routine as library function Ard Biesheuvel
2019-10-02 14:31   ` Greg KH
2019-10-04 13:36   ` Jason A. Donenfeld [this message]
2019-10-04 13:54     ` Ard Biesheuvel
2019-10-02 14:16 ` [PATCH v2 03/20] crypto: arm64/chacha - expose arm64 " Ard Biesheuvel
2019-10-02 14:31   ` Greg KH
2019-10-02 14:16 ` [PATCH v2 04/20] crypto: arm/chacha - expose ARM " Ard Biesheuvel
2019-10-04 13:52   ` Jason A. Donenfeld
2019-10-04 14:23     ` Ard Biesheuvel
2019-10-04 14:28       ` Jason A. Donenfeld
2019-10-04 14:29       ` Jason A. Donenfeld
2019-10-04 15:43         ` Eric Biggers
2019-10-04 15:24       ` Arnd Bergmann
2019-10-04 15:35         ` Ard Biesheuvel
2019-10-04 15:38           ` Jason A. Donenfeld
2019-10-02 14:16 ` [PATCH v2 05/20] crypto: mips/chacha - import accelerated 32r2 code from Zinc Ard Biesheuvel
2019-10-04 13:46   ` Jason A. Donenfeld
2019-10-04 14:38     ` Ard Biesheuvel
2019-10-04 14:38       ` Ard Biesheuvel
2019-10-04 14:59       ` Jason A. Donenfeld
2019-10-04 15:05         ` Ard Biesheuvel
2019-10-04 15:15         ` René van Dorst
2019-10-04 15:23           ` Ard Biesheuvel
2019-10-05  9:05             ` René van Dorst
2019-10-06 19:12             ` René van Dorst
2019-10-02 14:16 ` [PATCH v2 06/20] crypto: poly1305 - move into lib/crypto and refactor into library Ard Biesheuvel
2019-10-02 14:17 ` [PATCH v2 07/20] crypto: x86/poly1305 - expose existing driver as poly1305 library Ard Biesheuvel
2019-10-02 14:17 ` [PATCH v2 08/20] crypto: arm64/poly1305 - incorporate OpenSSL/CRYPTOGAMS NEON implementation Ard Biesheuvel
2019-10-02 14:17 ` [PATCH v2 09/20] crypto: arm/poly1305 " Ard Biesheuvel
2019-10-02 14:17 ` [PATCH v2 10/20] crypto: mips/poly1305 - import accelerated 32r2 code from Zinc Ard Biesheuvel
2019-10-04 13:48   ` Jason A. Donenfeld
2019-10-02 14:17 ` [PATCH v2 11/20] int128: move __uint128_t compiler test to Kconfig Ard Biesheuvel
2019-10-02 14:17 ` [PATCH v2 12/20] crypto: BLAKE2s - generic C library implementation and selftest Ard Biesheuvel
2019-10-02 14:17 ` [PATCH v2 13/20] crypto: BLAKE2s - x86_64 library implementation Ard Biesheuvel
2019-10-02 14:17 ` [PATCH v2 14/20] crypto: Curve25519 - generic C library implementations and selftest Ard Biesheuvel
2019-10-04 13:57   ` Jason A. Donenfeld
2019-10-04 14:03     ` Ard Biesheuvel
2019-10-02 14:17 ` [PATCH v2 15/20] crypto: lib/curve25519 - work around Clang stack spilling issue Ard Biesheuvel
2019-10-02 14:17 ` [PATCH v2 16/20] crypto: Curve25519 - x86_64 library implementation Ard Biesheuvel
2019-10-02 14:17 ` [PATCH v2 17/20] crypto: arm - import Bernstein and Schwabe's Curve25519 ARM implementation Ard Biesheuvel
2019-10-02 14:17 ` [PATCH v2 18/20] crypto: arm/Curve25519 - wire up NEON implementation Ard Biesheuvel
2019-10-04 14:00   ` Jason A. Donenfeld
2019-10-04 14:11     ` Ard Biesheuvel
2019-10-02 14:17 ` [PATCH v2 19/20] crypto: chacha20poly1305 - import construction and selftest from Zinc Ard Biesheuvel
2019-10-02 14:17 ` [PATCH v2 20/20] crypto: lib/chacha20poly1305 - reimplement crypt_from_sg() routine Ard Biesheuvel
2019-10-04 14:03   ` Jason A. Donenfeld
2019-10-04 14:07     ` Ard Biesheuvel
2019-10-03  8:43 ` [PATCH v2 00/20] crypto: crypto API library interfaces for WireGuard Ard Biesheuvel
2019-10-04 13:42   ` Jason A. Donenfeld
2019-10-04 13:52     ` Ard Biesheuvel
2019-10-04 14:53       ` Andy Lutomirski
2019-10-04 14:55         ` Jason A. Donenfeld
2019-10-04 14:59           ` Ard Biesheuvel
2019-10-04 14:56         ` Ard Biesheuvel
2019-10-05  7:24           ` Ard Biesheuvel
2019-10-07  4:44             ` Andy Lutomirski
2019-10-07  5:23               ` Ard Biesheuvel
2019-10-07 15:01                 ` Andy Lutomirski
2019-10-07 15:12                   ` Ard Biesheuvel
2019-10-07 16:05                     ` Andy Lutomirski
2019-10-04 14:50     ` Andy Lutomirski
2019-10-04 13:16 ` Jason A. Donenfeld
2019-10-04 14:12 ` Jason A. Donenfeld

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191004133649.GC112631@zx2c4.com \
    --to=jason@zx2c4.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=arnd@arndb.de \
    --cc=catalin.marinas@arm.com \
    --cc=dan.carpenter@oracle.com \
    --cc=davem@davemloft.net \
    --cc=ebiggers@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=jpoimboe@redhat.com \
    --cc=linux-crypto@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=martin@strongswan.org \
    --cc=maz@kernel.org \
    --cc=peterz@infradead.org \
    --cc=sneves@dei.uc.pt \
    --cc=torvalds@linux-foundation.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.