Re: [PATCH v2 0/8] add support for relative references in jump tables
From: Ard Biesheuvel
Date: Wed Jul 04 2018 - 04:50:11 EST
On 4 July 2018 at 09:59, Heiko Carstens <heiko.carstens@xxxxxxxxxx> wrote:
> On Mon, Jul 02, 2018 at 08:11:37PM +0200, Ard Biesheuvel wrote:
>> This series implements support for emitting the data structures associated
>> with jump tables as 32-bit relative references instead of absolute
>> references, which take up more space on builds that target 64-bit
>> architectures, or implement self relocation [or both].
>>
>> This series enables it for arm64 and x86, although other architectures
>> might benefit as well.
>
> Hello Ard,
>
> feel free to add the patch below which adds support for s390 to your series.
>
Thanks, I will incorporate it into v3 and beyond.
>> Changes since v1:
>> - change the relative reference to the static key to a 64-bit wide one on 64
>> bit architectures; this is necessary on arm64, which allows modules to
>> reside anywhere within a 4 GB window covering the core kernel text, which
>> means a 32-bit signed quantity with its +/- 2 GB range is insufficient.
>> Note that x86_64 changes are in preparation that widen the relocation
>> range as well (using the PIE linker), so I assumed that the same change
>> is appropriate for x86 as well.
>
> FWIW, kernel modules on s390 are since ages more than 2GB away from the
> core kernel text. So this is required for s390 as well.
>
OK, good to know.
--
Ard.
> From 77d87236f3d5474f33c25534d8ba2c7c54c88c55 Mon Sep 17 00:00:00 2001
> From: Heiko Carstens <heiko.carstens@xxxxxxxxxx>
> Date: Wed, 4 Jul 2018 09:13:37 +0200
> Subject: [PATCH] s390/jump_label: switch to relative references
>
> Signed-off-by: Heiko Carstens <heiko.carstens@xxxxxxxxxx>
> ---
> arch/s390/Kconfig | 1 +
> arch/s390/include/asm/jump_label.h | 40 +++++++++++++++-----------------------
> arch/s390/kernel/jump_label.c | 11 ++++++-----
> 3 files changed, 23 insertions(+), 29 deletions(-)
>
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index baed39772c84..0349fb06a2ec 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -121,6 +121,7 @@ config S390
> select HAVE_ALIGNED_STRUCT_PAGE if SLUB
> select HAVE_ARCH_AUDITSYSCALL
> select HAVE_ARCH_JUMP_LABEL
> + select HAVE_ARCH_JUMP_LABEL_RELATIVE
> select CPU_NO_EFFICIENT_FFS if !HAVE_MARCH_Z9_109_FEATURES
> select HAVE_ARCH_SECCOMP_FILTER
> select HAVE_ARCH_SOFT_DIRTY
> diff --git a/arch/s390/include/asm/jump_label.h b/arch/s390/include/asm/jump_label.h
> index 40f651292aa7..e2d3e6c43395 100644
> --- a/arch/s390/include/asm/jump_label.h
> +++ b/arch/s390/include/asm/jump_label.h
> @@ -14,41 +14,33 @@
> * We use a brcl 0,2 instruction for jump labels at compile time so it
> * can be easily distinguished from a hotpatch generated instruction.
> */
> -static __always_inline bool arch_static_branch(struct static_key *key, bool branch)
> +static inline bool arch_static_branch(struct static_key *key, bool branch)
> {
> - asm_volatile_goto("0: brcl 0,"__stringify(JUMP_LABEL_NOP_OFFSET)"\n"
> - ".pushsection __jump_table, \"aw\"\n"
> - ".balign 8\n"
> - ".quad 0b, %l[label], %0\n"
> - ".popsection\n"
> - : : "X" (&((char *)key)[branch]) : : label);
> -
> + asm_volatile_goto("0: brcl 0,"__stringify(JUMP_LABEL_NOP_OFFSET)"\n"
> + ".pushsection __jump_table,\"aw\"\n"
> + ".balign 8\n"
> + ".long 0b-.,%l[label]-.\n"
> + ".quad %0-.\n"
> + ".popsection\n"
> + : : "X" (&((char *)key)[branch]) : : label);
> return false;
> label:
> return true;
> }
>
> -static __always_inline bool arch_static_branch_jump(struct static_key *key, bool branch)
> +static inline bool arch_static_branch_jump(struct static_key *key, bool branch)
> {
> - asm_volatile_goto("0: brcl 15, %l[label]\n"
> - ".pushsection __jump_table, \"aw\"\n"
> - ".balign 8\n"
> - ".quad 0b, %l[label], %0\n"
> - ".popsection\n"
> - : : "X" (&((char *)key)[branch]) : : label);
> -
> + asm_volatile_goto("0: brcl 15,%l[label]\n"
> + ".pushsection __jump_table,\"aw\"\n"
> + ".balign 8\n"
> + ".long 0b-.,%l[label]-.\n"
> + ".quad %0-.\n"
> + ".popsection\n"
> + : : "X" (&((char *)key)[branch]) : : label);
> return false;
> label:
> return true;
> }
>
> -typedef unsigned long jump_label_t;
> -
> -struct jump_entry {
> - jump_label_t code;
> - jump_label_t target;
> - jump_label_t key;
> -};
> -
> #endif /* __ASSEMBLY__ */
> #endif
> diff --git a/arch/s390/kernel/jump_label.c b/arch/s390/kernel/jump_label.c
> index 43f8430fb67d..50a1798604a8 100644
> --- a/arch/s390/kernel/jump_label.c
> +++ b/arch/s390/kernel/jump_label.c
> @@ -33,13 +33,13 @@ static void jump_label_make_branch(struct jump_entry *entry, struct insn *insn)
> {
> /* brcl 15,offset */
> insn->opcode = 0xc0f4;
> - insn->offset = (entry->target - entry->code) >> 1;
> + insn->offset = (jump_entry_target(entry) - jump_entry_code(entry)) >> 1;
> }
>
> static void jump_label_bug(struct jump_entry *entry, struct insn *expected,
> struct insn *new)
> {
> - unsigned char *ipc = (unsigned char *)entry->code;
> + unsigned char *ipc = (unsigned char *)jump_entry_code(entry);
> unsigned char *ipe = (unsigned char *)expected;
> unsigned char *ipn = (unsigned char *)new;
>
> @@ -59,6 +59,7 @@ static void __jump_label_transform(struct jump_entry *entry,
> enum jump_label_type type,
> int init)
> {
> + void *code = (void *)jump_entry_code(entry);
> struct insn old, new;
>
> if (type == JUMP_LABEL_JMP) {
> @@ -69,13 +70,13 @@ static void __jump_label_transform(struct jump_entry *entry,
> jump_label_make_nop(entry, &new);
> }
> if (init) {
> - if (memcmp((void *)entry->code, &orignop, sizeof(orignop)))
> + if (memcmp(code, &orignop, sizeof(orignop)))
> jump_label_bug(entry, &orignop, &new);
> } else {
> - if (memcmp((void *)entry->code, &old, sizeof(old)))
> + if (memcmp(code, &old, sizeof(old)))
> jump_label_bug(entry, &old, &new);
> }
> - s390_kernel_write((void *)entry->code, &new, sizeof(new));
> + s390_kernel_write(code, &new, sizeof(new));
> }
>
> static int __sm_arch_jump_label_transform(void *data)
> --
> 2.16.4
>