All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@kernel.org>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Eric Biggers <ebiggers@kernel.org>,
	Samuel Neves <sneves@dei.uc.pt>,
	Andrew Lutomirski <luto@kernel.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	"David S. Miller" <davem@davemloft.net>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Kees Cook <keescook@chromium.org>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Richard
Subject: Re: [RFC PATCH 0/9] patchable function pointers for pluggable crypto routines
Date: Fri, 5 Oct 2018 10:26:08 -0700	[thread overview]
Message-ID: <CALCETrVxv1fHt6WE4ZpvVEwtfsKQ19gRrhhA40zwCXGMk+DTLQ@mail.gmail.com> (raw)
In-Reply-To: <CAKv+Gu-f4uKBF=gtFcNJEeoC2L4qShX0+3p7kVhjTn0tR+3Gng@mail.gmail.com>

On Fri, Oct 5, 2018 at 10:15 AM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>
> On 5 October 2018 at 15:37, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> ...
> > Therefore, I think this patch goes in exactly the wrong direction. I
> > mean, if you want to introduce dynamic patching as a means for making
> > the crypto API's dynamic dispatch stuff not as slow in a post-spectre
> > world, sure, go for it; that may very well be a good idea. But
> > presenting it as an alternative to Zinc very widely misses the point and
> > serves to prolong a series of bad design choices, which are now able to
> > be rectified by putting energy into Zinc instead.
> >
>
> This series has nothing to do with dynamic dispatch: the call sites
> call crypto functions using ordinary function calls (although my
> example uses CRC-T10DIF), and these calls are redirected via what is
> essentially a PLT entry, so that we can supsersede those routines at
> runtime.

If you really want to do it PLT-style, then just do:

extern void whatever_func(args);

Call it like:
whatever_func(args here);

And rig up something to emit asm like:

GLOBAL(whatever_func)
  jmpq default_whatever_func
ENDPROC(whatever_func)

Architectures without support can instead do:

void whatever_func(args)
{
  READ_ONCE(patchable_function_struct_for_whatever_func->ptr)(args);
}

and patch the asm function for basic support.  It will be slower than
necessary, but maybe the relocation trick could be used on top of this
to redirect the call to whatever_func directly to the target for
architectures that want to squeeze out the last bit of performance.
This might actually be the best of all worlds: easy implementation on
all architectures, no inline asm, and the totally non-magical version
works with okay performance.

(Is this what your code is doing?  I admit I didn't follow all the way
through all the macros.)

WARNING: multiple messages have this Message-ID (diff)
From: Andy Lutomirski <luto@kernel.org>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Eric Biggers <ebiggers@kernel.org>,
	Samuel Neves <sneves@dei.uc.pt>,
	Andrew Lutomirski <luto@kernel.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	"David S. Miller" <davem@davemloft.net>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Kees Cook <keescook@chromium.org>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Richard Weinberger <richard@nod.at>,
	Peter Zijlstra <peterz@infradead.org>,
	Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [RFC PATCH 0/9] patchable function pointers for pluggable crypto routines
Date: Fri, 5 Oct 2018 10:26:08 -0700	[thread overview]
Message-ID: <CALCETrVxv1fHt6WE4ZpvVEwtfsKQ19gRrhhA40zwCXGMk+DTLQ@mail.gmail.com> (raw)
In-Reply-To: <CAKv+Gu-f4uKBF=gtFcNJEeoC2L4qShX0+3p7kVhjTn0tR+3Gng@mail.gmail.com>

On Fri, Oct 5, 2018 at 10:15 AM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>
> On 5 October 2018 at 15:37, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> ...
> > Therefore, I think this patch goes in exactly the wrong direction. I
> > mean, if you want to introduce dynamic patching as a means for making
> > the crypto API's dynamic dispatch stuff not as slow in a post-spectre
> > world, sure, go for it; that may very well be a good idea. But
> > presenting it as an alternative to Zinc very widely misses the point and
> > serves to prolong a series of bad design choices, which are now able to
> > be rectified by putting energy into Zinc instead.
> >
>
> This series has nothing to do with dynamic dispatch: the call sites
> call crypto functions using ordinary function calls (although my
> example uses CRC-T10DIF), and these calls are redirected via what is
> essentially a PLT entry, so that we can supsersede those routines at
> runtime.

If you really want to do it PLT-style, then just do:

extern void whatever_func(args);

Call it like:
whatever_func(args here);

And rig up something to emit asm like:

GLOBAL(whatever_func)
  jmpq default_whatever_func
ENDPROC(whatever_func)

Architectures without support can instead do:

void whatever_func(args)
{
  READ_ONCE(patchable_function_struct_for_whatever_func->ptr)(args);
}

and patch the asm function for basic support.  It will be slower than
necessary, but maybe the relocation trick could be used on top of this
to redirect the call to whatever_func directly to the target for
architectures that want to squeeze out the last bit of performance.
This might actually be the best of all worlds: easy implementation on
all architectures, no inline asm, and the totally non-magical version
works with okay performance.

(Is this what your code is doing?  I admit I didn't follow all the way
through all the macros.)

WARNING: multiple messages have this Message-ID (diff)
From: Andy Lutomirski <luto@kernel.org>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Samuel Neves <sneves@dei.uc.pt>,
	Paul Mackerras <paulus@samba.org>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	Richard Weinberger <richard@nod.at>,
	Eric Biggers <ebiggers@kernel.org>,
	Ingo Molnar <mingo@redhat.com>, Kees Cook <keescook@chromium.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Andrew Lutomirski <luto@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [RFC PATCH 0/9] patchable function pointers for pluggable crypto routines
Date: Fri, 5 Oct 2018 10:26:08 -0700	[thread overview]
Message-ID: <CALCETrVxv1fHt6WE4ZpvVEwtfsKQ19gRrhhA40zwCXGMk+DTLQ@mail.gmail.com> (raw)
In-Reply-To: <CAKv+Gu-f4uKBF=gtFcNJEeoC2L4qShX0+3p7kVhjTn0tR+3Gng@mail.gmail.com>

On Fri, Oct 5, 2018 at 10:15 AM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>
> On 5 October 2018 at 15:37, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> ...
> > Therefore, I think this patch goes in exactly the wrong direction. I
> > mean, if you want to introduce dynamic patching as a means for making
> > the crypto API's dynamic dispatch stuff not as slow in a post-spectre
> > world, sure, go for it; that may very well be a good idea. But
> > presenting it as an alternative to Zinc very widely misses the point and
> > serves to prolong a series of bad design choices, which are now able to
> > be rectified by putting energy into Zinc instead.
> >
>
> This series has nothing to do with dynamic dispatch: the call sites
> call crypto functions using ordinary function calls (although my
> example uses CRC-T10DIF), and these calls are redirected via what is
> essentially a PLT entry, so that we can supsersede those routines at
> runtime.

If you really want to do it PLT-style, then just do:

extern void whatever_func(args);

Call it like:
whatever_func(args here);

And rig up something to emit asm like:

GLOBAL(whatever_func)
  jmpq default_whatever_func
ENDPROC(whatever_func)

Architectures without support can instead do:

void whatever_func(args)
{
  READ_ONCE(patchable_function_struct_for_whatever_func->ptr)(args);
}

and patch the asm function for basic support.  It will be slower than
necessary, but maybe the relocation trick could be used on top of this
to redirect the call to whatever_func directly to the target for
architectures that want to squeeze out the last bit of performance.
This might actually be the best of all worlds: easy implementation on
all architectures, no inline asm, and the totally non-magical version
works with okay performance.

(Is this what your code is doing?  I admit I didn't follow all the way
through all the macros.)

WARNING: multiple messages have this Message-ID (diff)
From: luto@kernel.org (Andy Lutomirski)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH 0/9] patchable function pointers for pluggable crypto routines
Date: Fri, 5 Oct 2018 10:26:08 -0700	[thread overview]
Message-ID: <CALCETrVxv1fHt6WE4ZpvVEwtfsKQ19gRrhhA40zwCXGMk+DTLQ@mail.gmail.com> (raw)
In-Reply-To: <CAKv+Gu-f4uKBF=gtFcNJEeoC2L4qShX0+3p7kVhjTn0tR+3Gng@mail.gmail.com>

On Fri, Oct 5, 2018 at 10:15 AM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>
> On 5 October 2018 at 15:37, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> ...
> > Therefore, I think this patch goes in exactly the wrong direction. I
> > mean, if you want to introduce dynamic patching as a means for making
> > the crypto API's dynamic dispatch stuff not as slow in a post-spectre
> > world, sure, go for it; that may very well be a good idea. But
> > presenting it as an alternative to Zinc very widely misses the point and
> > serves to prolong a series of bad design choices, which are now able to
> > be rectified by putting energy into Zinc instead.
> >
>
> This series has nothing to do with dynamic dispatch: the call sites
> call crypto functions using ordinary function calls (although my
> example uses CRC-T10DIF), and these calls are redirected via what is
> essentially a PLT entry, so that we can supsersede those routines at
> runtime.

If you really want to do it PLT-style, then just do:

extern void whatever_func(args);

Call it like:
whatever_func(args here);

And rig up something to emit asm like:

GLOBAL(whatever_func)
  jmpq default_whatever_func
ENDPROC(whatever_func)

Architectures without support can instead do:

void whatever_func(args)
{
  READ_ONCE(patchable_function_struct_for_whatever_func->ptr)(args);
}

and patch the asm function for basic support.  It will be slower than
necessary, but maybe the relocation trick could be used on top of this
to redirect the call to whatever_func directly to the target for
architectures that want to squeeze out the last bit of performance.
This might actually be the best of all worlds: easy implementation on
all architectures, no inline asm, and the totally non-magical version
works with okay performance.

(Is this what your code is doing?  I admit I didn't follow all the way
through all the macros.)

  reply	other threads:[~2018-10-05 17:26 UTC|newest]

Thread overview: 120+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-05  8:13 [RFC PATCH 0/9] patchable function pointers for pluggable crypto routines Ard Biesheuvel
2018-10-05  8:13 ` Ard Biesheuvel
2018-10-05  8:13 ` Ard Biesheuvel
2018-10-05  8:13 ` Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 1/9] kernel: add support for patchable function pointers Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05 13:57   ` Peter Zijlstra
2018-10-05 13:57     ` Peter Zijlstra
2018-10-05 13:57     ` Peter Zijlstra
2018-10-05 13:57     ` Peter Zijlstra
2018-10-05 14:03     ` Ard Biesheuvel
2018-10-05 14:03       ` Ard Biesheuvel
2018-10-05 14:03       ` Ard Biesheuvel
2018-10-05 14:03       ` Ard Biesheuvel
2018-10-05 14:14   ` Peter Zijlstra
2018-10-05 14:14     ` Peter Zijlstra
2018-10-05 14:14     ` Peter Zijlstra
2018-10-05 14:14     ` Peter Zijlstra
2018-10-05 14:57     ` Ard Biesheuvel
2018-10-05 14:57       ` Ard Biesheuvel
2018-10-05 14:57       ` Ard Biesheuvel
2018-10-05 14:57       ` Ard Biesheuvel
2018-10-05 15:08     ` Andy Lutomirski
2018-10-05 15:08       ` Andy Lutomirski
2018-10-05 15:08       ` Andy Lutomirski
2018-10-05 15:08       ` Andy Lutomirski
2018-10-05 15:24       ` Ard Biesheuvel
2018-10-05 15:24         ` Ard Biesheuvel
2018-10-05 15:24         ` Ard Biesheuvel
2018-10-05 15:24         ` Ard Biesheuvel
2018-10-05 16:58         ` Andy Lutomirski
2018-10-05 16:58           ` Andy Lutomirski
2018-10-05 16:58           ` Andy Lutomirski
2018-10-05 16:58           ` Andy Lutomirski
2018-10-05 17:11           ` Ard Biesheuvel
2018-10-05 17:11             ` Ard Biesheuvel
2018-10-05 17:11             ` Ard Biesheuvel
2018-10-05 17:11             ` Ard Biesheuvel
2018-10-05 17:20             ` Andy Lutomirski
2018-10-05 17:20               ` Andy Lutomirski
2018-10-05 17:20               ` Andy Lutomirski
2018-10-05 17:20               ` Andy Lutomirski
2018-10-05 17:23               ` Ard Biesheuvel
2018-10-05 17:23                 ` Ard Biesheuvel
2018-10-05 17:23                 ` Ard Biesheuvel
2018-10-05 17:23                 ` Ard Biesheuvel
2018-10-05 17:28                 ` Andy Lutomirski
2018-10-05 17:28                   ` Andy Lutomirski
2018-10-05 17:28                   ` Andy Lutomirski
2018-10-05 17:28                   ` Andy Lutomirski
2018-10-05 17:37                   ` Jason A. Donenfeld
2018-10-05 17:37                     ` Jason A. Donenfeld
2018-10-05 17:37                     ` Jason A. Donenfeld
2018-10-05 17:37                     ` Jason A. Donenfeld
2018-10-05 17:44                     ` Andy Lutomirski
2018-10-05 17:44                       ` Andy Lutomirski
2018-10-05 17:44                       ` Andy Lutomirski
2018-10-05 17:44                       ` Andy Lutomirski
2018-10-05 18:28                       ` Jason A. Donenfeld
2018-10-05 18:28                         ` Jason A. Donenfeld
2018-10-05 18:28                         ` Jason A. Donenfeld
2018-10-05 18:28                         ` Jason A. Donenfeld
2018-10-05 19:13                         ` Ard Biesheuvel
2018-10-05 19:13                           ` Ard Biesheuvel
2018-10-05 19:13                           ` Ard Biesheuvel
2018-10-05 19:13                           ` Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 2/9] arm64: kernel: add arch " Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 3/9] crypto: crc-t10dif - make crc_t10dif a static inline Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 4/9] crypto: crc-t10dif - use patchable function pointer for core update routine Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 5/9] crypto: crc-t10dif/arm64 - move PMULL based code into core library Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 6/9] crypto: crc-t10dif/arm " Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 7/9] crypto: crct10dif/generic - switch crypto API driver to " Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 8/9] crypto: crc-t10dif/powerpc - move PMULL based code into " Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 9/9] crypto: crc-t10dif/x86 " Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05  8:13   ` Ard Biesheuvel
2018-10-05 13:37 ` [RFC PATCH 0/9] patchable function pointers for pluggable crypto routines Jason A. Donenfeld
2018-10-05 13:37   ` Jason A. Donenfeld
2018-10-05 13:37   ` Jason A. Donenfeld
2018-10-05 13:37   ` Jason A. Donenfeld
2018-10-05 17:15   ` Ard Biesheuvel
2018-10-05 17:15     ` Ard Biesheuvel
2018-10-05 17:15     ` Ard Biesheuvel
2018-10-05 17:15     ` Ard Biesheuvel
2018-10-05 17:26     ` Andy Lutomirski [this message]
2018-10-05 17:26       ` Andy Lutomirski
2018-10-05 17:26       ` Andy Lutomirski
2018-10-05 17:26       ` Andy Lutomirski
2018-10-05 17:28       ` Ard Biesheuvel
2018-10-05 17:28         ` Ard Biesheuvel
2018-10-05 17:28         ` Ard Biesheuvel
2018-10-05 17:28         ` Ard Biesheuvel
2018-10-05 18:00         ` Andy Lutomirski
2018-10-05 18:00           ` Andy Lutomirski
2018-10-05 18:00           ` Andy Lutomirski
2018-10-05 18:00           ` Andy Lutomirski

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=CALCETrVxv1fHt6WE4ZpvVEwtfsKQ19gRrhhA40zwCXGMk+DTLQ@mail.gmail.com \
    --to=luto@kernel.org \
    --cc=Jason@zx2c4.com \
    --cc=akpm@linux-foundation.org \
    --cc=ard.biesheuvel@linaro.org \
    --cc=arnd@arndb.de \
    --cc=benh@kernel.crashing.org \
    --cc=catalin.marinas@arm.com \
    --cc=davem@davemloft.net \
    --cc=ebiggers@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=mingo@redhat.com \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.org \
    --cc=sneves@dei.uc.pt \
    --cc=tglx@linutronix.de \
    --cc=will.deacon@arm.com \
    /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.