linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: luto@amacapital.net (Andy Lutomirski)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH 1/9] kernel: add support for patchable function pointers
Date: Fri, 5 Oct 2018 08:08:08 -0700	[thread overview]
Message-ID: <9E0E08C8-0DFC-4E50-A4FA-73208835EF9E@amacapital.net> (raw)
In-Reply-To: <20181005141433.GS19272@hirez.programming.kicks-ass.net>



> On Oct 5, 2018, at 7:14 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> 
>> On Fri, Oct 05, 2018 at 10:13:25AM +0200, Ard Biesheuvel wrote:
>> diff --git a/include/linux/ffp.h b/include/linux/ffp.h
>> new file mode 100644
>> index 000000000000..8fc3b4c9b38f
>> --- /dev/null
>> +++ b/include/linux/ffp.h
>> @@ -0,0 +1,43 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +
>> +#ifndef __LINUX_FFP_H
>> +#define __LINUX_FFP_H
>> +
>> +#include <linux/types.h>
>> +#include <linux/compiler.h>
>> +
>> +#ifdef CONFIG_HAVE_ARCH_FFP
>> +#include <asm/ffp.h>
>> +#else
>> +
>> +struct ffp {
>> +    void    (**fn)(void);
>> +    void    (*default_fn)(void);
>> +};
>> +
>> +#define DECLARE_FFP(_fn, _def)                        \
>> +    extern typeof(_def) *_fn;                    \
>> +    extern struct ffp const __ffp_ ## _fn
>> +
>> +#define DEFINE_FFP(_fn, _def)                        \
>> +    typeof(_def) *_fn = &_def;                    \
>> +    struct ffp const __ffp_ ## _fn                    \
>> +        = { (void(**)(void))&_fn, (void(*)(void))&_def };    \
>> +    EXPORT_SYMBOL(__ffp_ ## _fn)
>> +
>> +static inline void ffp_set_target(const struct ffp *m, void *new_fn)
>> +{
>> +    WRITE_ONCE(*m->fn, new_fn);
>> +}
>> +
>> +static inline void ffp_reset_target(const struct ffp *m)
>> +{
>> +    WRITE_ONCE(*m->fn, m->default_fn);
>> +}
>> +
>> +#endif
>> +
>> +#define SET_FFP(_fn, _new)    ffp_set_target(&__ffp_ ## _fn, _new)
>> +#define RESET_FFP(_fn)        ffp_reset_target(&__ffp_ ## _fn)
>> +
>> +#endif
> 
> I don't understand this interface. There is no wrapper for the call
> site, so how are we going to patch all call-sites when you update the
> target?

I?m also confused.

Anyway, we have patchable functions on x86. They?re called PVOPs, and they?re way overcomplicated.

I?ve proposed a better way that should generate better code, be more portable, and be more maintainable.  It goes like this.

To call the function, you literally just call  the default implementation.  It *might* be necessary to call a nonexistent wrapper to avoid annoying optimizations. At build time, the kernel is built with relocations, so the object files contain relocation entries for the call. We collect these entries into a table. If we?re using the ?nonexistent wrapper? approach, we can link in a .S or linker script to alias them to the default implementation.

To patch them, we just patch them. It can?t necessarily be done concurrently because nothing forces the right alignment. But we can do it at boot time and module load time. (Maybe we can patch at runtime on architectures with appropriate instruction alignment.  Or we ask gcc for an extension to align calls to a function.)

Most of the machinery already exists: this is roughly how the module loader resolves calls outside of a module.

  parent reply	other threads:[~2018-10-05 15:08 UTC|newest]

Thread overview: 30+ 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 ` [RFC PATCH 1/9] kernel: add support for patchable function pointers Ard Biesheuvel
2018-10-05 13:57   ` Peter Zijlstra
2018-10-05 14:03     ` Ard Biesheuvel
2018-10-05 14:14   ` Peter Zijlstra
2018-10-05 14:57     ` Ard Biesheuvel
2018-10-05 15:08     ` Andy Lutomirski [this message]
2018-10-05 15:24       ` Ard Biesheuvel
2018-10-05 16:58         ` Andy Lutomirski
2018-10-05 17:11           ` Ard Biesheuvel
2018-10-05 17:20             ` Andy Lutomirski
2018-10-05 17:23               ` Ard Biesheuvel
2018-10-05 17:28                 ` Andy Lutomirski
2018-10-05 17:37                   ` Jason A. Donenfeld
2018-10-05 17:44                     ` Andy Lutomirski
2018-10-05 18:28                       ` Jason A. Donenfeld
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 ` [RFC PATCH 3/9] crypto: crc-t10dif - make crc_t10dif a static inline 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 ` [RFC PATCH 5/9] crypto: crc-t10dif/arm64 - move PMULL based code into core library Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 6/9] crypto: crc-t10dif/arm " 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 ` [RFC PATCH 8/9] crypto: crc-t10dif/powerpc - move PMULL based code into " Ard Biesheuvel
2018-10-05  8:13 ` [RFC PATCH 9/9] crypto: crc-t10dif/x86 " Ard Biesheuvel
2018-10-05 13:37 ` [RFC PATCH 0/9] patchable function pointers for pluggable crypto routines Jason A. Donenfeld
2018-10-05 17:15   ` Ard Biesheuvel
2018-10-05 17:26     ` Andy Lutomirski
2018-10-05 17:28       ` Ard Biesheuvel
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=9E0E08C8-0DFC-4E50-A4FA-73208835EF9E@amacapital.net \
    --to=luto@amacapital.net \
    --cc=linux-arm-kernel@lists.infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).