* [PATCH v6 0/7] Optimize jump label implementation for ARM64
@ 2013-12-10 16:03 ` Jiang Liu
0 siblings, 0 replies; 42+ messages in thread
From: Jiang Liu @ 2013-12-10 16:03 UTC (permalink / raw)
To: linux-arm-kernel
This patchset tries to optimize arch specfic jump label implementation
for ARM64 by dynamic kernel text patching.
To enable this feature, your toolchain must support "asm goto" extension
and "%c" constraint extesion. Current GCC for AARCH64 doesn't support
"%c", and there's a patch for it now:
http://gcc.gnu.org/ml/gcc-patches/2013-10/msg01314.html
It has been tested on ARM Fast mode and a real hardware platform.
Any comments are welcomed!
V5->V6:
1) optimize stop_machine() related code
2) simplify aarch64_insn_patch_text_nosync() interface
3) refine comments
V4->V5:
1) rework the stop_machine() related code
2) rework aarch64_insn_gen_nop()
V3->V4:
1) resolve a race condition in kernel text patching
2) address other review comments
V2->V3:
1) fix a bug in comparing signed and unsigned values
2) detect big endian by checking __AARCH64EB__
V1->V2: address review comments of V1
1) refine comments
2) add a new interface to always synchronize with stop_machine()
when patching code
3) handle endian issue when patching code
Jiang Liu (7):
arm64: introduce basic aarch64 instruction decoding helpers
arm64: introduce interfaces to hotpatch kernel and module code
arm64: move encode_insn_immediate() from module.c to insn.c
arm64: introduce aarch64_insn_gen_{nop|branch_imm}() helper functions
arm64, jump label: detect %c support for ARM64
arm64, jump label: optimize jump label implementation
jump_label: use defined macros instead of hard-coding for better
readability
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/insn.h | 108 +++++++++++++
arch/arm64/include/asm/jump_label.h | 51 ++++++
arch/arm64/kernel/Makefile | 3 +-
arch/arm64/kernel/insn.c | 302 ++++++++++++++++++++++++++++++++++++
arch/arm64/kernel/jump_label.c | 59 +++++++
arch/arm64/kernel/module.c | 157 ++++++-------------
include/linux/jump_label.h | 15 +-
scripts/gcc-goto.sh | 2 +-
9 files changed, 581 insertions(+), 117 deletions(-)
create mode 100644 arch/arm64/include/asm/insn.h
create mode 100644 arch/arm64/include/asm/jump_label.h
create mode 100644 arch/arm64/kernel/insn.c
create mode 100644 arch/arm64/kernel/jump_label.c
--
1.8.1.2
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH v6 0/7] Optimize jump label implementation for ARM64 @ 2013-12-10 16:03 ` Jiang Liu 0 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-10 16:03 UTC (permalink / raw) To: Steven Rostedt, Catalin Marinas, Will Deacon, Sandeepa Prabhu, AKASHI Takahiro Cc: Jiang Liu, linux-arm-kernel, linux-kernel This patchset tries to optimize arch specfic jump label implementation for ARM64 by dynamic kernel text patching. To enable this feature, your toolchain must support "asm goto" extension and "%c" constraint extesion. Current GCC for AARCH64 doesn't support "%c", and there's a patch for it now: http://gcc.gnu.org/ml/gcc-patches/2013-10/msg01314.html It has been tested on ARM Fast mode and a real hardware platform. Any comments are welcomed! V5->V6: 1) optimize stop_machine() related code 2) simplify aarch64_insn_patch_text_nosync() interface 3) refine comments V4->V5: 1) rework the stop_machine() related code 2) rework aarch64_insn_gen_nop() V3->V4: 1) resolve a race condition in kernel text patching 2) address other review comments V2->V3: 1) fix a bug in comparing signed and unsigned values 2) detect big endian by checking __AARCH64EB__ V1->V2: address review comments of V1 1) refine comments 2) add a new interface to always synchronize with stop_machine() when patching code 3) handle endian issue when patching code Jiang Liu (7): arm64: introduce basic aarch64 instruction decoding helpers arm64: introduce interfaces to hotpatch kernel and module code arm64: move encode_insn_immediate() from module.c to insn.c arm64: introduce aarch64_insn_gen_{nop|branch_imm}() helper functions arm64, jump label: detect %c support for ARM64 arm64, jump label: optimize jump label implementation jump_label: use defined macros instead of hard-coding for better readability arch/arm64/Kconfig | 1 + arch/arm64/include/asm/insn.h | 108 +++++++++++++ arch/arm64/include/asm/jump_label.h | 51 ++++++ arch/arm64/kernel/Makefile | 3 +- arch/arm64/kernel/insn.c | 302 ++++++++++++++++++++++++++++++++++++ arch/arm64/kernel/jump_label.c | 59 +++++++ arch/arm64/kernel/module.c | 157 ++++++------------- include/linux/jump_label.h | 15 +- scripts/gcc-goto.sh | 2 +- 9 files changed, 581 insertions(+), 117 deletions(-) create mode 100644 arch/arm64/include/asm/insn.h create mode 100644 arch/arm64/include/asm/jump_label.h create mode 100644 arch/arm64/kernel/insn.c create mode 100644 arch/arm64/kernel/jump_label.c -- 1.8.1.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v6 1/7] arm64: introduce basic aarch64 instruction decoding helpers 2013-12-10 16:03 ` Jiang Liu @ 2013-12-10 16:03 ` Jiang Liu -1 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-10 16:03 UTC (permalink / raw) To: linux-arm-kernel Introduce basic aarch64 instruction decoding helper aarch64_get_insn_class() and aarch64_insn_hotpatch_safe(). Reviewed-by: Will Deacon <will.deacon@arm.com> Signed-off-by: Jiang Liu <liuj97@gmail.com> --- arch/arm64/include/asm/insn.h | 77 ++++++++++++++++++++++++++++++++++++ arch/arm64/kernel/Makefile | 2 +- arch/arm64/kernel/insn.c | 91 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 arch/arm64/include/asm/insn.h create mode 100644 arch/arm64/kernel/insn.c diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h new file mode 100644 index 0000000..1bdc44c --- /dev/null +++ b/arch/arm64/include/asm/insn.h @@ -0,0 +1,77 @@ +/* + * Copyright (C) 2013 Huawei Ltd. + * Author: Jiang Liu <liuj97@gmail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#ifndef __ASM_INSN_H +#define __ASM_INSN_H +#include <linux/types.h> + +/* + * ARM Architecture Reference Manual for ARMv8 Profile-A, Issue A.a + * Section C3.1 "A64 instruction index by encoding": + * AArch64 main encoding table + * Bit position + * 28 27 26 25 Encoding Group + * 0 0 - - Unallocated + * 1 0 0 - Data processing, immediate + * 1 0 1 - Branch, exception generation and system instructions + * - 1 - 0 Loads and stores + * - 1 0 1 Data processing - register + * 0 1 1 1 Data processing - SIMD and floating point + * 1 1 1 1 Data processing - SIMD and floating point + * "-" means "don't care" + */ +enum aarch64_insn_encoding_class { + AARCH64_INSN_CLS_UNKNOWN, /* UNALLOCATED */ + AARCH64_INSN_CLS_DP_IMM, /* Data processing - immediate */ + AARCH64_INSN_CLS_DP_REG, /* Data processing - register */ + AARCH64_INSN_CLS_DP_FPSIMD, /* Data processing - SIMD and FP */ + AARCH64_INSN_CLS_LDST, /* Loads and stores */ + AARCH64_INSN_CLS_BR_SYS, /* Branch, exception generation and + * system instructions */ +}; + +enum aarch64_insn_hint_op { + AARCH64_INSN_HINT_NOP = 0x0 << 5, + AARCH64_INSN_HINT_YIELD = 0x1 << 5, + AARCH64_INSN_HINT_WFE = 0x2 << 5, + AARCH64_INSN_HINT_WFI = 0x3 << 5, + AARCH64_INSN_HINT_SEV = 0x4 << 5, + AARCH64_INSN_HINT_SEVL = 0x5 << 5, +}; + +#define __AARCH64_INSN_FUNCS(abbr, mask, val) \ +static __always_inline bool aarch64_insn_is_##abbr(u32 code) \ +{ return (code & (mask)) == (val); } \ +static __always_inline u32 aarch64_insn_get_##abbr##_value(void) \ +{ return (val); } + +__AARCH64_INSN_FUNCS(b, 0xFC000000, 0x14000000) +__AARCH64_INSN_FUNCS(bl, 0xFC000000, 0x94000000) +__AARCH64_INSN_FUNCS(svc, 0xFFE0001F, 0xD4000001) +__AARCH64_INSN_FUNCS(hvc, 0xFFE0001F, 0xD4000002) +__AARCH64_INSN_FUNCS(smc, 0xFFE0001F, 0xD4000003) +__AARCH64_INSN_FUNCS(brk, 0xFFE0001F, 0xD4200000) +__AARCH64_INSN_FUNCS(hint, 0xFFFFF01F, 0xD503201F) + +#undef __AARCH64_INSN_FUNCS + +bool aarch64_insn_is_nop(u32 insn); + +enum aarch64_insn_encoding_class aarch64_get_insn_class(u32 insn); + +bool aarch64_insn_hotpatch_safe(u32 old_insn, u32 new_insn); + +#endif /* __ASM_INSN_H */ diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index 5ba2fd4..cd95a25 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -9,7 +9,7 @@ AFLAGS_head.o := -DTEXT_OFFSET=$(TEXT_OFFSET) arm64-obj-y := cputable.o debug-monitors.o entry.o irq.o fpsimd.o \ entry-fpsimd.o process.o ptrace.o setup.o signal.o \ sys.o stacktrace.o time.o traps.o io.o vdso.o \ - hyp-stub.o psci.o cpu_ops.o + hyp-stub.o psci.o cpu_ops.o insn.o arm64-obj-$(CONFIG_COMPAT) += sys32.o kuser32.o signal32.o \ sys_compat.o diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c new file mode 100644 index 0000000..56a2498 --- /dev/null +++ b/arch/arm64/kernel/insn.c @@ -0,0 +1,91 @@ +/* + * Copyright (C) 2013 Huawei Ltd. + * Author: Jiang Liu <liuj97@gmail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#include <linux/compiler.h> +#include <linux/kernel.h> +#include <asm/insn.h> + +static int aarch64_insn_encoding_class[] = { + AARCH64_INSN_CLS_UNKNOWN, + AARCH64_INSN_CLS_UNKNOWN, + AARCH64_INSN_CLS_UNKNOWN, + AARCH64_INSN_CLS_UNKNOWN, + AARCH64_INSN_CLS_LDST, + AARCH64_INSN_CLS_DP_REG, + AARCH64_INSN_CLS_LDST, + AARCH64_INSN_CLS_DP_FPSIMD, + AARCH64_INSN_CLS_DP_IMM, + AARCH64_INSN_CLS_DP_IMM, + AARCH64_INSN_CLS_BR_SYS, + AARCH64_INSN_CLS_BR_SYS, + AARCH64_INSN_CLS_LDST, + AARCH64_INSN_CLS_DP_REG, + AARCH64_INSN_CLS_LDST, + AARCH64_INSN_CLS_DP_FPSIMD, +}; + +enum aarch64_insn_encoding_class __kprobes aarch64_get_insn_class(u32 insn) +{ + return aarch64_insn_encoding_class[(insn >> 25) & 0xf]; +} + +/* NOP is an alias of HINT */ +bool __kprobes aarch64_insn_is_nop(u32 insn) +{ + if (!aarch64_insn_is_hint(insn)) + return false; + + switch (insn & 0xFE0) { + case AARCH64_INSN_HINT_YIELD: + case AARCH64_INSN_HINT_WFE: + case AARCH64_INSN_HINT_WFI: + case AARCH64_INSN_HINT_SEV: + case AARCH64_INSN_HINT_SEVL: + return false; + default: + return true; + } +} + +static bool __kprobes __aarch64_insn_hotpatch_safe(u32 insn) +{ + if (aarch64_get_insn_class(insn) != AARCH64_INSN_CLS_BR_SYS) + return false; + + return aarch64_insn_is_b(insn) || + aarch64_insn_is_bl(insn) || + aarch64_insn_is_svc(insn) || + aarch64_insn_is_hvc(insn) || + aarch64_insn_is_smc(insn) || + aarch64_insn_is_brk(insn) || + aarch64_insn_is_nop(insn); +} + +/* + * ARM Architecture Reference Manual for ARMv8 Profile-A, Issue A.a + * Section B2.6.5 "Concurrent modification and execution of instructions": + * Concurrent modification and execution of instructions can lead to the + * resulting instruction performing any behavior that can be achieved by + * executing any sequence of instructions that can be executed from the + * same Exception level, except where the instruction before modification + * and the instruction after modification is a B, BL, NOP, BKPT, SVC, HVC, + * or SMC instruction. + */ +bool __kprobes aarch64_insn_hotpatch_safe(u32 old_insn, u32 new_insn) +{ + return __aarch64_insn_hotpatch_safe(old_insn) && + __aarch64_insn_hotpatch_safe(new_insn); +} -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v6 1/7] arm64: introduce basic aarch64 instruction decoding helpers @ 2013-12-10 16:03 ` Jiang Liu 0 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-10 16:03 UTC (permalink / raw) To: Steven Rostedt, Catalin Marinas, Will Deacon, Sandeepa Prabhu, AKASHI Takahiro, Marc Zyngier, Mark Rutland, Arnd Bergmann, linux-arm-kernel, linux-kernel Cc: Jiang Liu Introduce basic aarch64 instruction decoding helper aarch64_get_insn_class() and aarch64_insn_hotpatch_safe(). Reviewed-by: Will Deacon <will.deacon@arm.com> Signed-off-by: Jiang Liu <liuj97@gmail.com> --- arch/arm64/include/asm/insn.h | 77 ++++++++++++++++++++++++++++++++++++ arch/arm64/kernel/Makefile | 2 +- arch/arm64/kernel/insn.c | 91 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 arch/arm64/include/asm/insn.h create mode 100644 arch/arm64/kernel/insn.c diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h new file mode 100644 index 0000000..1bdc44c --- /dev/null +++ b/arch/arm64/include/asm/insn.h @@ -0,0 +1,77 @@ +/* + * Copyright (C) 2013 Huawei Ltd. + * Author: Jiang Liu <liuj97@gmail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#ifndef __ASM_INSN_H +#define __ASM_INSN_H +#include <linux/types.h> + +/* + * ARM Architecture Reference Manual for ARMv8 Profile-A, Issue A.a + * Section C3.1 "A64 instruction index by encoding": + * AArch64 main encoding table + * Bit position + * 28 27 26 25 Encoding Group + * 0 0 - - Unallocated + * 1 0 0 - Data processing, immediate + * 1 0 1 - Branch, exception generation and system instructions + * - 1 - 0 Loads and stores + * - 1 0 1 Data processing - register + * 0 1 1 1 Data processing - SIMD and floating point + * 1 1 1 1 Data processing - SIMD and floating point + * "-" means "don't care" + */ +enum aarch64_insn_encoding_class { + AARCH64_INSN_CLS_UNKNOWN, /* UNALLOCATED */ + AARCH64_INSN_CLS_DP_IMM, /* Data processing - immediate */ + AARCH64_INSN_CLS_DP_REG, /* Data processing - register */ + AARCH64_INSN_CLS_DP_FPSIMD, /* Data processing - SIMD and FP */ + AARCH64_INSN_CLS_LDST, /* Loads and stores */ + AARCH64_INSN_CLS_BR_SYS, /* Branch, exception generation and + * system instructions */ +}; + +enum aarch64_insn_hint_op { + AARCH64_INSN_HINT_NOP = 0x0 << 5, + AARCH64_INSN_HINT_YIELD = 0x1 << 5, + AARCH64_INSN_HINT_WFE = 0x2 << 5, + AARCH64_INSN_HINT_WFI = 0x3 << 5, + AARCH64_INSN_HINT_SEV = 0x4 << 5, + AARCH64_INSN_HINT_SEVL = 0x5 << 5, +}; + +#define __AARCH64_INSN_FUNCS(abbr, mask, val) \ +static __always_inline bool aarch64_insn_is_##abbr(u32 code) \ +{ return (code & (mask)) == (val); } \ +static __always_inline u32 aarch64_insn_get_##abbr##_value(void) \ +{ return (val); } + +__AARCH64_INSN_FUNCS(b, 0xFC000000, 0x14000000) +__AARCH64_INSN_FUNCS(bl, 0xFC000000, 0x94000000) +__AARCH64_INSN_FUNCS(svc, 0xFFE0001F, 0xD4000001) +__AARCH64_INSN_FUNCS(hvc, 0xFFE0001F, 0xD4000002) +__AARCH64_INSN_FUNCS(smc, 0xFFE0001F, 0xD4000003) +__AARCH64_INSN_FUNCS(brk, 0xFFE0001F, 0xD4200000) +__AARCH64_INSN_FUNCS(hint, 0xFFFFF01F, 0xD503201F) + +#undef __AARCH64_INSN_FUNCS + +bool aarch64_insn_is_nop(u32 insn); + +enum aarch64_insn_encoding_class aarch64_get_insn_class(u32 insn); + +bool aarch64_insn_hotpatch_safe(u32 old_insn, u32 new_insn); + +#endif /* __ASM_INSN_H */ diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index 5ba2fd4..cd95a25 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -9,7 +9,7 @@ AFLAGS_head.o := -DTEXT_OFFSET=$(TEXT_OFFSET) arm64-obj-y := cputable.o debug-monitors.o entry.o irq.o fpsimd.o \ entry-fpsimd.o process.o ptrace.o setup.o signal.o \ sys.o stacktrace.o time.o traps.o io.o vdso.o \ - hyp-stub.o psci.o cpu_ops.o + hyp-stub.o psci.o cpu_ops.o insn.o arm64-obj-$(CONFIG_COMPAT) += sys32.o kuser32.o signal32.o \ sys_compat.o diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c new file mode 100644 index 0000000..56a2498 --- /dev/null +++ b/arch/arm64/kernel/insn.c @@ -0,0 +1,91 @@ +/* + * Copyright (C) 2013 Huawei Ltd. + * Author: Jiang Liu <liuj97@gmail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#include <linux/compiler.h> +#include <linux/kernel.h> +#include <asm/insn.h> + +static int aarch64_insn_encoding_class[] = { + AARCH64_INSN_CLS_UNKNOWN, + AARCH64_INSN_CLS_UNKNOWN, + AARCH64_INSN_CLS_UNKNOWN, + AARCH64_INSN_CLS_UNKNOWN, + AARCH64_INSN_CLS_LDST, + AARCH64_INSN_CLS_DP_REG, + AARCH64_INSN_CLS_LDST, + AARCH64_INSN_CLS_DP_FPSIMD, + AARCH64_INSN_CLS_DP_IMM, + AARCH64_INSN_CLS_DP_IMM, + AARCH64_INSN_CLS_BR_SYS, + AARCH64_INSN_CLS_BR_SYS, + AARCH64_INSN_CLS_LDST, + AARCH64_INSN_CLS_DP_REG, + AARCH64_INSN_CLS_LDST, + AARCH64_INSN_CLS_DP_FPSIMD, +}; + +enum aarch64_insn_encoding_class __kprobes aarch64_get_insn_class(u32 insn) +{ + return aarch64_insn_encoding_class[(insn >> 25) & 0xf]; +} + +/* NOP is an alias of HINT */ +bool __kprobes aarch64_insn_is_nop(u32 insn) +{ + if (!aarch64_insn_is_hint(insn)) + return false; + + switch (insn & 0xFE0) { + case AARCH64_INSN_HINT_YIELD: + case AARCH64_INSN_HINT_WFE: + case AARCH64_INSN_HINT_WFI: + case AARCH64_INSN_HINT_SEV: + case AARCH64_INSN_HINT_SEVL: + return false; + default: + return true; + } +} + +static bool __kprobes __aarch64_insn_hotpatch_safe(u32 insn) +{ + if (aarch64_get_insn_class(insn) != AARCH64_INSN_CLS_BR_SYS) + return false; + + return aarch64_insn_is_b(insn) || + aarch64_insn_is_bl(insn) || + aarch64_insn_is_svc(insn) || + aarch64_insn_is_hvc(insn) || + aarch64_insn_is_smc(insn) || + aarch64_insn_is_brk(insn) || + aarch64_insn_is_nop(insn); +} + +/* + * ARM Architecture Reference Manual for ARMv8 Profile-A, Issue A.a + * Section B2.6.5 "Concurrent modification and execution of instructions": + * Concurrent modification and execution of instructions can lead to the + * resulting instruction performing any behavior that can be achieved by + * executing any sequence of instructions that can be executed from the + * same Exception level, except where the instruction before modification + * and the instruction after modification is a B, BL, NOP, BKPT, SVC, HVC, + * or SMC instruction. + */ +bool __kprobes aarch64_insn_hotpatch_safe(u32 old_insn, u32 new_insn) +{ + return __aarch64_insn_hotpatch_safe(old_insn) && + __aarch64_insn_hotpatch_safe(new_insn); +} -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v6 2/7] arm64: introduce interfaces to hotpatch kernel and module code 2013-12-10 16:03 ` Jiang Liu @ 2013-12-10 16:03 ` Jiang Liu -1 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-10 16:03 UTC (permalink / raw) To: linux-arm-kernel Introduce three interfaces to patch kernel and module code: aarch64_insn_patch_text_nosync(): patch code without synchronization, it's caller's responsibility to synchronize all CPUs if needed. aarch64_insn_patch_text_sync(): patch code and always synchronize with stop_machine() aarch64_insn_patch_text(): patch code and synchronize with stop_machine() if needed Signed-off-by: Jiang Liu <liuj97@gmail.com> --- arch/arm64/include/asm/insn.h | 10 +++- arch/arm64/kernel/insn.c | 117 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 1 deletion(-) diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h index 1bdc44c..bf8085f 100644 --- a/arch/arm64/include/asm/insn.h +++ b/arch/arm64/include/asm/insn.h @@ -18,6 +18,9 @@ #define __ASM_INSN_H #include <linux/types.h> +/* A64 instructions are always 32 bits. */ +#define AARCH64_INSN_SIZE 4 + /* * ARM Architecture Reference Manual for ARMv8 Profile-A, Issue A.a * Section C3.1 "A64 instruction index by encoding": @@ -70,8 +73,13 @@ __AARCH64_INSN_FUNCS(hint, 0xFFFFF01F, 0xD503201F) bool aarch64_insn_is_nop(u32 insn); +int aarch64_insn_read(void *addr, u32 *insnp); +int aarch64_insn_write(void *addr, u32 insn); enum aarch64_insn_encoding_class aarch64_get_insn_class(u32 insn); - bool aarch64_insn_hotpatch_safe(u32 old_insn, u32 new_insn); +int aarch64_insn_patch_text_nosync(void *addr, u32 insn); +int aarch64_insn_patch_text_sync(void *addrs[], u32 insns[], int cnt); +int aarch64_insn_patch_text(void *addrs[], u32 insns[], int cnt); + #endif /* __ASM_INSN_H */ diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c index 56a2498..c1b002a8 100644 --- a/arch/arm64/kernel/insn.c +++ b/arch/arm64/kernel/insn.c @@ -16,6 +16,10 @@ */ #include <linux/compiler.h> #include <linux/kernel.h> +#include <linux/smp.h> +#include <linux/stop_machine.h> +#include <linux/uaccess.h> +#include <asm/cacheflush.h> #include <asm/insn.h> static int aarch64_insn_encoding_class[] = { @@ -60,6 +64,28 @@ bool __kprobes aarch64_insn_is_nop(u32 insn) } } +/* + * In ARMv8-A, A64 instructions have a fixed length of 32 bits and are always + * little-endian. + */ +int __kprobes aarch64_insn_read(void *addr, u32 *insnp) +{ + int ret; + u32 val; + + ret = probe_kernel_read(&val, addr, AARCH64_INSN_SIZE); + if (!ret) + *insnp = le32_to_cpu(val); + + return ret; +} + +int __kprobes aarch64_insn_write(void *addr, u32 insn) +{ + insn = cpu_to_le32(insn); + return probe_kernel_write(addr, &insn, AARCH64_INSN_SIZE); +} + static bool __kprobes __aarch64_insn_hotpatch_safe(u32 insn) { if (aarch64_get_insn_class(insn) != AARCH64_INSN_CLS_BR_SYS) @@ -89,3 +115,94 @@ bool __kprobes aarch64_insn_hotpatch_safe(u32 old_insn, u32 new_insn) return __aarch64_insn_hotpatch_safe(old_insn) && __aarch64_insn_hotpatch_safe(new_insn); } + +int __kprobes aarch64_insn_patch_text_nosync(void *addr, u32 insn) +{ + u32 *tp = addr; + int ret; + + /* A64 instructions must be word aligned */ + if ((uintptr_t)tp & 0x3) + return -EINVAL; + + ret = aarch64_insn_write(tp, insn); + if (ret == 0) + flush_icache_range((uintptr_t)tp, + (uintptr_t)tp + AARCH64_INSN_SIZE); + + return ret; +} + +struct aarch64_insn_patch { + void **text_addrs; + u32 *new_insns; + int insn_cnt; + atomic_t cpu_count; +}; + +static int __kprobes aarch64_insn_patch_text_cb(void *arg) +{ + int i, ret = 0; + struct aarch64_insn_patch *pp = arg; + + /* The first CPU becomes master */ + if (atomic_inc_return(&pp->cpu_count) == 1) { + for (i = 0; ret == 0 && i < pp->insn_cnt; i++) + ret = aarch64_insn_patch_text_nosync(pp->text_addrs[i], + pp->new_insns[i]); + /* Make sure changes are visible to other CPUs and let them continue */ + smp_wmb(); + atomic_set(&pp->cpu_count, -1); + } else { + while (atomic_read(&pp->cpu_count) != -1) + cpu_relax(); + smp_rmb(); + isb(); + } + + return ret; +} + +int __kprobes aarch64_insn_patch_text_sync(void *addrs[], u32 insns[], int cnt) +{ + struct aarch64_insn_patch patch = { + .text_addrs = addrs, + .new_insns = insns, + .insn_cnt = cnt, + .cpu_count = ATOMIC_INIT(0), + }; + + if (cnt <= 0) + return -EINVAL; + + return stop_machine(aarch64_insn_patch_text_cb, &patch, + cpu_online_mask); +} + +int __kprobes aarch64_insn_patch_text(void *addrs[], u32 insns[], int cnt) +{ + int ret; + u32 insn; + + /* Unsafe to patch multiple instructions without synchronizaiton */ + if (cnt == 1) { + ret = aarch64_insn_read(addrs[0], &insn); + if (ret) + return ret; + + if (aarch64_insn_hotpatch_safe(insn, insns[0])) { + /* + * ARMv8 architecture doesn't guarantee all CPUs see + * the new instruction after returning from function + * aarch64_insn_patch_text_nosync(). So send IPIs to + * all other CPUs to achieve instruction + * synchronization. + */ + ret = aarch64_insn_patch_text_nosync(addrs[0], insns[0]); + kick_all_cpus_sync(); + return ret; + } + } + + return aarch64_insn_patch_text_sync(addrs, insns, cnt); +} -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v6 2/7] arm64: introduce interfaces to hotpatch kernel and module code @ 2013-12-10 16:03 ` Jiang Liu 0 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-10 16:03 UTC (permalink / raw) To: Steven Rostedt, Catalin Marinas, Will Deacon, Sandeepa Prabhu, AKASHI Takahiro, linux-arm-kernel, linux-kernel Cc: Jiang Liu Introduce three interfaces to patch kernel and module code: aarch64_insn_patch_text_nosync(): patch code without synchronization, it's caller's responsibility to synchronize all CPUs if needed. aarch64_insn_patch_text_sync(): patch code and always synchronize with stop_machine() aarch64_insn_patch_text(): patch code and synchronize with stop_machine() if needed Signed-off-by: Jiang Liu <liuj97@gmail.com> --- arch/arm64/include/asm/insn.h | 10 +++- arch/arm64/kernel/insn.c | 117 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 1 deletion(-) diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h index 1bdc44c..bf8085f 100644 --- a/arch/arm64/include/asm/insn.h +++ b/arch/arm64/include/asm/insn.h @@ -18,6 +18,9 @@ #define __ASM_INSN_H #include <linux/types.h> +/* A64 instructions are always 32 bits. */ +#define AARCH64_INSN_SIZE 4 + /* * ARM Architecture Reference Manual for ARMv8 Profile-A, Issue A.a * Section C3.1 "A64 instruction index by encoding": @@ -70,8 +73,13 @@ __AARCH64_INSN_FUNCS(hint, 0xFFFFF01F, 0xD503201F) bool aarch64_insn_is_nop(u32 insn); +int aarch64_insn_read(void *addr, u32 *insnp); +int aarch64_insn_write(void *addr, u32 insn); enum aarch64_insn_encoding_class aarch64_get_insn_class(u32 insn); - bool aarch64_insn_hotpatch_safe(u32 old_insn, u32 new_insn); +int aarch64_insn_patch_text_nosync(void *addr, u32 insn); +int aarch64_insn_patch_text_sync(void *addrs[], u32 insns[], int cnt); +int aarch64_insn_patch_text(void *addrs[], u32 insns[], int cnt); + #endif /* __ASM_INSN_H */ diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c index 56a2498..c1b002a8 100644 --- a/arch/arm64/kernel/insn.c +++ b/arch/arm64/kernel/insn.c @@ -16,6 +16,10 @@ */ #include <linux/compiler.h> #include <linux/kernel.h> +#include <linux/smp.h> +#include <linux/stop_machine.h> +#include <linux/uaccess.h> +#include <asm/cacheflush.h> #include <asm/insn.h> static int aarch64_insn_encoding_class[] = { @@ -60,6 +64,28 @@ bool __kprobes aarch64_insn_is_nop(u32 insn) } } +/* + * In ARMv8-A, A64 instructions have a fixed length of 32 bits and are always + * little-endian. + */ +int __kprobes aarch64_insn_read(void *addr, u32 *insnp) +{ + int ret; + u32 val; + + ret = probe_kernel_read(&val, addr, AARCH64_INSN_SIZE); + if (!ret) + *insnp = le32_to_cpu(val); + + return ret; +} + +int __kprobes aarch64_insn_write(void *addr, u32 insn) +{ + insn = cpu_to_le32(insn); + return probe_kernel_write(addr, &insn, AARCH64_INSN_SIZE); +} + static bool __kprobes __aarch64_insn_hotpatch_safe(u32 insn) { if (aarch64_get_insn_class(insn) != AARCH64_INSN_CLS_BR_SYS) @@ -89,3 +115,94 @@ bool __kprobes aarch64_insn_hotpatch_safe(u32 old_insn, u32 new_insn) return __aarch64_insn_hotpatch_safe(old_insn) && __aarch64_insn_hotpatch_safe(new_insn); } + +int __kprobes aarch64_insn_patch_text_nosync(void *addr, u32 insn) +{ + u32 *tp = addr; + int ret; + + /* A64 instructions must be word aligned */ + if ((uintptr_t)tp & 0x3) + return -EINVAL; + + ret = aarch64_insn_write(tp, insn); + if (ret == 0) + flush_icache_range((uintptr_t)tp, + (uintptr_t)tp + AARCH64_INSN_SIZE); + + return ret; +} + +struct aarch64_insn_patch { + void **text_addrs; + u32 *new_insns; + int insn_cnt; + atomic_t cpu_count; +}; + +static int __kprobes aarch64_insn_patch_text_cb(void *arg) +{ + int i, ret = 0; + struct aarch64_insn_patch *pp = arg; + + /* The first CPU becomes master */ + if (atomic_inc_return(&pp->cpu_count) == 1) { + for (i = 0; ret == 0 && i < pp->insn_cnt; i++) + ret = aarch64_insn_patch_text_nosync(pp->text_addrs[i], + pp->new_insns[i]); + /* Make sure changes are visible to other CPUs and let them continue */ + smp_wmb(); + atomic_set(&pp->cpu_count, -1); + } else { + while (atomic_read(&pp->cpu_count) != -1) + cpu_relax(); + smp_rmb(); + isb(); + } + + return ret; +} + +int __kprobes aarch64_insn_patch_text_sync(void *addrs[], u32 insns[], int cnt) +{ + struct aarch64_insn_patch patch = { + .text_addrs = addrs, + .new_insns = insns, + .insn_cnt = cnt, + .cpu_count = ATOMIC_INIT(0), + }; + + if (cnt <= 0) + return -EINVAL; + + return stop_machine(aarch64_insn_patch_text_cb, &patch, + cpu_online_mask); +} + +int __kprobes aarch64_insn_patch_text(void *addrs[], u32 insns[], int cnt) +{ + int ret; + u32 insn; + + /* Unsafe to patch multiple instructions without synchronizaiton */ + if (cnt == 1) { + ret = aarch64_insn_read(addrs[0], &insn); + if (ret) + return ret; + + if (aarch64_insn_hotpatch_safe(insn, insns[0])) { + /* + * ARMv8 architecture doesn't guarantee all CPUs see + * the new instruction after returning from function + * aarch64_insn_patch_text_nosync(). So send IPIs to + * all other CPUs to achieve instruction + * synchronization. + */ + ret = aarch64_insn_patch_text_nosync(addrs[0], insns[0]); + kick_all_cpus_sync(); + return ret; + } + } + + return aarch64_insn_patch_text_sync(addrs, insns, cnt); +} -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v6 2/7] arm64: introduce interfaces to hotpatch kernel and module code 2013-12-10 16:03 ` Jiang Liu @ 2013-12-13 15:54 ` Will Deacon -1 siblings, 0 replies; 42+ messages in thread From: Will Deacon @ 2013-12-13 15:54 UTC (permalink / raw) To: linux-arm-kernel On Tue, Dec 10, 2013 at 04:03:48PM +0000, Jiang Liu wrote: > Introduce three interfaces to patch kernel and module code: > aarch64_insn_patch_text_nosync(): > patch code without synchronization, it's caller's responsibility > to synchronize all CPUs if needed. > aarch64_insn_patch_text_sync(): > patch code and always synchronize with stop_machine() > aarch64_insn_patch_text(): > patch code and synchronize with stop_machine() if needed > > Signed-off-by: Jiang Liu <liuj97@gmail.com> > --- > arch/arm64/include/asm/insn.h | 10 +++- > arch/arm64/kernel/insn.c | 117 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 126 insertions(+), 1 deletion(-) Two comments on the patching callback... > +static int __kprobes aarch64_insn_patch_text_cb(void *arg) > +{ > + int i, ret = 0; > + struct aarch64_insn_patch *pp = arg; > + > + /* The first CPU becomes master */ > + if (atomic_inc_return(&pp->cpu_count) == 1) { > + for (i = 0; ret == 0 && i < pp->insn_cnt; i++) > + ret = aarch64_insn_patch_text_nosync(pp->text_addrs[i], > + pp->new_insns[i]); > + /* Make sure changes are visible to other CPUs and let them continue */ > + smp_wmb(); As I mentioned last time, you can rely on aarch64_insn_patch_text_nosync to enforce ordering (flush_icache_range ends with dsb; isb). Just add a comment saying that. > + atomic_set(&pp->cpu_count, -1); > + } else { > + while (atomic_read(&pp->cpu_count) != -1) > + cpu_relax(); > + smp_rmb(); You don't need the smp_rmb here. Will ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v6 2/7] arm64: introduce interfaces to hotpatch kernel and module code @ 2013-12-13 15:54 ` Will Deacon 0 siblings, 0 replies; 42+ messages in thread From: Will Deacon @ 2013-12-13 15:54 UTC (permalink / raw) To: Jiang Liu Cc: Steven Rostedt, Catalin Marinas, Sandeepa Prabhu, AKASHI Takahiro, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org On Tue, Dec 10, 2013 at 04:03:48PM +0000, Jiang Liu wrote: > Introduce three interfaces to patch kernel and module code: > aarch64_insn_patch_text_nosync(): > patch code without synchronization, it's caller's responsibility > to synchronize all CPUs if needed. > aarch64_insn_patch_text_sync(): > patch code and always synchronize with stop_machine() > aarch64_insn_patch_text(): > patch code and synchronize with stop_machine() if needed > > Signed-off-by: Jiang Liu <liuj97@gmail.com> > --- > arch/arm64/include/asm/insn.h | 10 +++- > arch/arm64/kernel/insn.c | 117 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 126 insertions(+), 1 deletion(-) Two comments on the patching callback... > +static int __kprobes aarch64_insn_patch_text_cb(void *arg) > +{ > + int i, ret = 0; > + struct aarch64_insn_patch *pp = arg; > + > + /* The first CPU becomes master */ > + if (atomic_inc_return(&pp->cpu_count) == 1) { > + for (i = 0; ret == 0 && i < pp->insn_cnt; i++) > + ret = aarch64_insn_patch_text_nosync(pp->text_addrs[i], > + pp->new_insns[i]); > + /* Make sure changes are visible to other CPUs and let them continue */ > + smp_wmb(); As I mentioned last time, you can rely on aarch64_insn_patch_text_nosync to enforce ordering (flush_icache_range ends with dsb; isb). Just add a comment saying that. > + atomic_set(&pp->cpu_count, -1); > + } else { > + while (atomic_read(&pp->cpu_count) != -1) > + cpu_relax(); > + smp_rmb(); You don't need the smp_rmb here. Will ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v6 3/7] arm64: move encode_insn_immediate() from module.c to insn.c 2013-12-10 16:03 ` Jiang Liu @ 2013-12-10 16:03 ` Jiang Liu -1 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-10 16:03 UTC (permalink / raw) To: linux-arm-kernel Function encode_insn_immediate() will be used by other instruction manipulate related functions, so move it into insn.c and rename it as aarch64_insn_encode_immediate(). Reviewed-by: Will Deacon <will.deacon@arm.com> Signed-off-by: Jiang Liu <liuj97@gmail.com> --- arch/arm64/include/asm/insn.h | 13 ++++ arch/arm64/kernel/insn.c | 54 +++++++++++++++ arch/arm64/kernel/module.c | 157 +++++++++++++----------------------------- 3 files changed, 114 insertions(+), 110 deletions(-) diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h index bf8085f..fb44660 100644 --- a/arch/arm64/include/asm/insn.h +++ b/arch/arm64/include/asm/insn.h @@ -55,6 +55,17 @@ enum aarch64_insn_hint_op { AARCH64_INSN_HINT_SEVL = 0x5 << 5, }; +enum aarch64_insn_imm_type { + AARCH64_INSN_IMM_ADR, + AARCH64_INSN_IMM_26, + AARCH64_INSN_IMM_19, + AARCH64_INSN_IMM_16, + AARCH64_INSN_IMM_14, + AARCH64_INSN_IMM_12, + AARCH64_INSN_IMM_9, + AARCH64_INSN_IMM_MAX +}; + #define __AARCH64_INSN_FUNCS(abbr, mask, val) \ static __always_inline bool aarch64_insn_is_##abbr(u32 code) \ { return (code & (mask)) == (val); } \ @@ -76,6 +87,8 @@ bool aarch64_insn_is_nop(u32 insn); int aarch64_insn_read(void *addr, u32 *insnp); int aarch64_insn_write(void *addr, u32 insn); enum aarch64_insn_encoding_class aarch64_get_insn_class(u32 insn); +u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type, + u32 insn, u64 imm); bool aarch64_insn_hotpatch_safe(u32 old_insn, u32 new_insn); int aarch64_insn_patch_text_nosync(void *addr, u32 insn); diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c index c1b002a8..03e9a5f 100644 --- a/arch/arm64/kernel/insn.c +++ b/arch/arm64/kernel/insn.c @@ -206,3 +206,57 @@ int __kprobes aarch64_insn_patch_text(void *addrs[], u32 insns[], int cnt) return aarch64_insn_patch_text_sync(addrs, insns, cnt); } + +u32 __kprobes aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type, + u32 insn, u64 imm) +{ + u32 immlo, immhi, lomask, himask, mask; + int shift; + + switch (type) { + case AARCH64_INSN_IMM_ADR: + lomask = 0x3; + himask = 0x7ffff; + immlo = imm & lomask; + imm >>= 2; + immhi = imm & himask; + imm = (immlo << 24) | (immhi); + mask = (lomask << 24) | (himask); + shift = 5; + break; + case AARCH64_INSN_IMM_26: + mask = BIT(26) - 1; + shift = 0; + break; + case AARCH64_INSN_IMM_19: + mask = BIT(19) - 1; + shift = 5; + break; + case AARCH64_INSN_IMM_16: + mask = BIT(16) - 1; + shift = 5; + break; + case AARCH64_INSN_IMM_14: + mask = BIT(14) - 1; + shift = 5; + break; + case AARCH64_INSN_IMM_12: + mask = BIT(12) - 1; + shift = 10; + break; + case AARCH64_INSN_IMM_9: + mask = BIT(9) - 1; + shift = 12; + break; + default: + pr_err("aarch64_insn_encode_immediate: unknown immediate encoding %d\n", + type); + return 0; + } + + /* Update the immediate field. */ + insn &= ~(mask << shift); + insn |= (imm & mask) << shift; + + return insn; +} diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c index e2ad0d8..1eb1cc9 100644 --- a/arch/arm64/kernel/module.c +++ b/arch/arm64/kernel/module.c @@ -25,6 +25,10 @@ #include <linux/mm.h> #include <linux/moduleloader.h> #include <linux/vmalloc.h> +#include <asm/insn.h> + +#define AARCH64_INSN_IMM_MOVNZ AARCH64_INSN_IMM_MAX +#define AARCH64_INSN_IMM_MOVK AARCH64_INSN_IMM_16 void *module_alloc(unsigned long size) { @@ -94,28 +98,18 @@ static int reloc_data(enum aarch64_reloc_op op, void *place, u64 val, int len) return 0; } -enum aarch64_imm_type { - INSN_IMM_MOVNZ, - INSN_IMM_MOVK, - INSN_IMM_ADR, - INSN_IMM_26, - INSN_IMM_19, - INSN_IMM_16, - INSN_IMM_14, - INSN_IMM_12, - INSN_IMM_9, -}; - -static u32 encode_insn_immediate(enum aarch64_imm_type type, u32 insn, u64 imm) +static int reloc_insn_movw(enum aarch64_reloc_op op, void *place, u64 val, + int lsb, enum aarch64_insn_imm_type imm_type) { - u32 immlo, immhi, lomask, himask, mask; - int shift; + u64 imm, limit = 0; + s64 sval; + u32 insn = le32_to_cpu(*(u32 *)place); - /* The instruction stream is always little endian. */ - insn = le32_to_cpu(insn); + sval = do_reloc(op, place, val); + sval >>= lsb; + imm = sval & 0xffff; - switch (type) { - case INSN_IMM_MOVNZ: + if (imm_type == AARCH64_INSN_IMM_MOVNZ) { /* * For signed MOVW relocations, we have to manipulate the * instruction encoding depending on whether or not the @@ -134,70 +128,12 @@ static u32 encode_insn_immediate(enum aarch64_imm_type type, u32 insn, u64 imm) */ imm = ~imm; } - case INSN_IMM_MOVK: - mask = BIT(16) - 1; - shift = 5; - break; - case INSN_IMM_ADR: - lomask = 0x3; - himask = 0x7ffff; - immlo = imm & lomask; - imm >>= 2; - immhi = imm & himask; - imm = (immlo << 24) | (immhi); - mask = (lomask << 24) | (himask); - shift = 5; - break; - case INSN_IMM_26: - mask = BIT(26) - 1; - shift = 0; - break; - case INSN_IMM_19: - mask = BIT(19) - 1; - shift = 5; - break; - case INSN_IMM_16: - mask = BIT(16) - 1; - shift = 5; - break; - case INSN_IMM_14: - mask = BIT(14) - 1; - shift = 5; - break; - case INSN_IMM_12: - mask = BIT(12) - 1; - shift = 10; - break; - case INSN_IMM_9: - mask = BIT(9) - 1; - shift = 12; - break; - default: - pr_err("encode_insn_immediate: unknown immediate encoding %d\n", - type); - return 0; + imm_type = AARCH64_INSN_IMM_MOVK; } - /* Update the immediate field. */ - insn &= ~(mask << shift); - insn |= (imm & mask) << shift; - - return cpu_to_le32(insn); -} - -static int reloc_insn_movw(enum aarch64_reloc_op op, void *place, u64 val, - int lsb, enum aarch64_imm_type imm_type) -{ - u64 imm, limit = 0; - s64 sval; - u32 insn = *(u32 *)place; - - sval = do_reloc(op, place, val); - sval >>= lsb; - imm = sval & 0xffff; - /* Update the instruction with the new encoding. */ - *(u32 *)place = encode_insn_immediate(imm_type, insn, imm); + insn = aarch64_insn_encode_immediate(imm_type, insn, imm); + *(u32 *)place = cpu_to_le32(insn); /* Shift out the immediate field. */ sval >>= 16; @@ -206,9 +142,9 @@ static int reloc_insn_movw(enum aarch64_reloc_op op, void *place, u64 val, * For unsigned immediates, the overflow check is straightforward. * For signed immediates, the sign bit is actually the bit past the * most significant bit of the field. - * The INSN_IMM_16 immediate type is unsigned. + * The AARCH64_INSN_IMM_16 immediate type is unsigned. */ - if (imm_type != INSN_IMM_16) { + if (imm_type != AARCH64_INSN_IMM_16) { sval++; limit++; } @@ -221,11 +157,11 @@ static int reloc_insn_movw(enum aarch64_reloc_op op, void *place, u64 val, } static int reloc_insn_imm(enum aarch64_reloc_op op, void *place, u64 val, - int lsb, int len, enum aarch64_imm_type imm_type) + int lsb, int len, enum aarch64_insn_imm_type imm_type) { u64 imm, imm_mask; s64 sval; - u32 insn = *(u32 *)place; + u32 insn = le32_to_cpu(*(u32 *)place); /* Calculate the relocation value. */ sval = do_reloc(op, place, val); @@ -236,7 +172,8 @@ static int reloc_insn_imm(enum aarch64_reloc_op op, void *place, u64 val, imm = sval & imm_mask; /* Update the instruction's immediate field. */ - *(u32 *)place = encode_insn_immediate(imm_type, insn, imm); + insn = aarch64_insn_encode_immediate(imm_type, insn, imm); + *(u32 *)place = cpu_to_le32(insn); /* * Extract the upper value bits (including the sign bit) and @@ -318,125 +255,125 @@ int apply_relocate_add(Elf64_Shdr *sechdrs, overflow_check = false; case R_AARCH64_MOVW_UABS_G0: ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 0, - INSN_IMM_16); + AARCH64_INSN_IMM_16); break; case R_AARCH64_MOVW_UABS_G1_NC: overflow_check = false; case R_AARCH64_MOVW_UABS_G1: ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 16, - INSN_IMM_16); + AARCH64_INSN_IMM_16); break; case R_AARCH64_MOVW_UABS_G2_NC: overflow_check = false; case R_AARCH64_MOVW_UABS_G2: ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 32, - INSN_IMM_16); + AARCH64_INSN_IMM_16); break; case R_AARCH64_MOVW_UABS_G3: /* We're using the top bits so we can't overflow. */ overflow_check = false; ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 48, - INSN_IMM_16); + AARCH64_INSN_IMM_16); break; case R_AARCH64_MOVW_SABS_G0: ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 0, - INSN_IMM_MOVNZ); + AARCH64_INSN_IMM_MOVNZ); break; case R_AARCH64_MOVW_SABS_G1: ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 16, - INSN_IMM_MOVNZ); + AARCH64_INSN_IMM_MOVNZ); break; case R_AARCH64_MOVW_SABS_G2: ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 32, - INSN_IMM_MOVNZ); + AARCH64_INSN_IMM_MOVNZ); break; case R_AARCH64_MOVW_PREL_G0_NC: overflow_check = false; ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 0, - INSN_IMM_MOVK); + AARCH64_INSN_IMM_MOVK); break; case R_AARCH64_MOVW_PREL_G0: ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 0, - INSN_IMM_MOVNZ); + AARCH64_INSN_IMM_MOVNZ); break; case R_AARCH64_MOVW_PREL_G1_NC: overflow_check = false; ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 16, - INSN_IMM_MOVK); + AARCH64_INSN_IMM_MOVK); break; case R_AARCH64_MOVW_PREL_G1: ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 16, - INSN_IMM_MOVNZ); + AARCH64_INSN_IMM_MOVNZ); break; case R_AARCH64_MOVW_PREL_G2_NC: overflow_check = false; ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 32, - INSN_IMM_MOVK); + AARCH64_INSN_IMM_MOVK); break; case R_AARCH64_MOVW_PREL_G2: ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 32, - INSN_IMM_MOVNZ); + AARCH64_INSN_IMM_MOVNZ); break; case R_AARCH64_MOVW_PREL_G3: /* We're using the top bits so we can't overflow. */ overflow_check = false; ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 48, - INSN_IMM_MOVNZ); + AARCH64_INSN_IMM_MOVNZ); break; /* Immediate instruction relocations. */ case R_AARCH64_LD_PREL_LO19: ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 2, 19, - INSN_IMM_19); + AARCH64_INSN_IMM_19); break; case R_AARCH64_ADR_PREL_LO21: ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 0, 21, - INSN_IMM_ADR); + AARCH64_INSN_IMM_ADR); break; case R_AARCH64_ADR_PREL_PG_HI21_NC: overflow_check = false; case R_AARCH64_ADR_PREL_PG_HI21: ovf = reloc_insn_imm(RELOC_OP_PAGE, loc, val, 12, 21, - INSN_IMM_ADR); + AARCH64_INSN_IMM_ADR); break; case R_AARCH64_ADD_ABS_LO12_NC: case R_AARCH64_LDST8_ABS_LO12_NC: overflow_check = false; ovf = reloc_insn_imm(RELOC_OP_ABS, loc, val, 0, 12, - INSN_IMM_12); + AARCH64_INSN_IMM_12); break; case R_AARCH64_LDST16_ABS_LO12_NC: overflow_check = false; ovf = reloc_insn_imm(RELOC_OP_ABS, loc, val, 1, 11, - INSN_IMM_12); + AARCH64_INSN_IMM_12); break; case R_AARCH64_LDST32_ABS_LO12_NC: overflow_check = false; ovf = reloc_insn_imm(RELOC_OP_ABS, loc, val, 2, 10, - INSN_IMM_12); + AARCH64_INSN_IMM_12); break; case R_AARCH64_LDST64_ABS_LO12_NC: overflow_check = false; ovf = reloc_insn_imm(RELOC_OP_ABS, loc, val, 3, 9, - INSN_IMM_12); + AARCH64_INSN_IMM_12); break; case R_AARCH64_LDST128_ABS_LO12_NC: overflow_check = false; ovf = reloc_insn_imm(RELOC_OP_ABS, loc, val, 4, 8, - INSN_IMM_12); + AARCH64_INSN_IMM_12); break; case R_AARCH64_TSTBR14: ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 2, 14, - INSN_IMM_14); + AARCH64_INSN_IMM_14); break; case R_AARCH64_CONDBR19: ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 2, 19, - INSN_IMM_19); + AARCH64_INSN_IMM_19); break; case R_AARCH64_JUMP26: case R_AARCH64_CALL26: ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 2, 26, - INSN_IMM_26); + AARCH64_INSN_IMM_26); break; default: -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v6 3/7] arm64: move encode_insn_immediate() from module.c to insn.c @ 2013-12-10 16:03 ` Jiang Liu 0 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-10 16:03 UTC (permalink / raw) To: Steven Rostedt, Catalin Marinas, Will Deacon, Sandeepa Prabhu, AKASHI Takahiro, Andrew Morton, Jianguo Wu, David Rientjes, linux-arm-kernel, linux-kernel Cc: Jiang Liu Function encode_insn_immediate() will be used by other instruction manipulate related functions, so move it into insn.c and rename it as aarch64_insn_encode_immediate(). Reviewed-by: Will Deacon <will.deacon@arm.com> Signed-off-by: Jiang Liu <liuj97@gmail.com> --- arch/arm64/include/asm/insn.h | 13 ++++ arch/arm64/kernel/insn.c | 54 +++++++++++++++ arch/arm64/kernel/module.c | 157 +++++++++++++----------------------------- 3 files changed, 114 insertions(+), 110 deletions(-) diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h index bf8085f..fb44660 100644 --- a/arch/arm64/include/asm/insn.h +++ b/arch/arm64/include/asm/insn.h @@ -55,6 +55,17 @@ enum aarch64_insn_hint_op { AARCH64_INSN_HINT_SEVL = 0x5 << 5, }; +enum aarch64_insn_imm_type { + AARCH64_INSN_IMM_ADR, + AARCH64_INSN_IMM_26, + AARCH64_INSN_IMM_19, + AARCH64_INSN_IMM_16, + AARCH64_INSN_IMM_14, + AARCH64_INSN_IMM_12, + AARCH64_INSN_IMM_9, + AARCH64_INSN_IMM_MAX +}; + #define __AARCH64_INSN_FUNCS(abbr, mask, val) \ static __always_inline bool aarch64_insn_is_##abbr(u32 code) \ { return (code & (mask)) == (val); } \ @@ -76,6 +87,8 @@ bool aarch64_insn_is_nop(u32 insn); int aarch64_insn_read(void *addr, u32 *insnp); int aarch64_insn_write(void *addr, u32 insn); enum aarch64_insn_encoding_class aarch64_get_insn_class(u32 insn); +u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type, + u32 insn, u64 imm); bool aarch64_insn_hotpatch_safe(u32 old_insn, u32 new_insn); int aarch64_insn_patch_text_nosync(void *addr, u32 insn); diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c index c1b002a8..03e9a5f 100644 --- a/arch/arm64/kernel/insn.c +++ b/arch/arm64/kernel/insn.c @@ -206,3 +206,57 @@ int __kprobes aarch64_insn_patch_text(void *addrs[], u32 insns[], int cnt) return aarch64_insn_patch_text_sync(addrs, insns, cnt); } + +u32 __kprobes aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type, + u32 insn, u64 imm) +{ + u32 immlo, immhi, lomask, himask, mask; + int shift; + + switch (type) { + case AARCH64_INSN_IMM_ADR: + lomask = 0x3; + himask = 0x7ffff; + immlo = imm & lomask; + imm >>= 2; + immhi = imm & himask; + imm = (immlo << 24) | (immhi); + mask = (lomask << 24) | (himask); + shift = 5; + break; + case AARCH64_INSN_IMM_26: + mask = BIT(26) - 1; + shift = 0; + break; + case AARCH64_INSN_IMM_19: + mask = BIT(19) - 1; + shift = 5; + break; + case AARCH64_INSN_IMM_16: + mask = BIT(16) - 1; + shift = 5; + break; + case AARCH64_INSN_IMM_14: + mask = BIT(14) - 1; + shift = 5; + break; + case AARCH64_INSN_IMM_12: + mask = BIT(12) - 1; + shift = 10; + break; + case AARCH64_INSN_IMM_9: + mask = BIT(9) - 1; + shift = 12; + break; + default: + pr_err("aarch64_insn_encode_immediate: unknown immediate encoding %d\n", + type); + return 0; + } + + /* Update the immediate field. */ + insn &= ~(mask << shift); + insn |= (imm & mask) << shift; + + return insn; +} diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c index e2ad0d8..1eb1cc9 100644 --- a/arch/arm64/kernel/module.c +++ b/arch/arm64/kernel/module.c @@ -25,6 +25,10 @@ #include <linux/mm.h> #include <linux/moduleloader.h> #include <linux/vmalloc.h> +#include <asm/insn.h> + +#define AARCH64_INSN_IMM_MOVNZ AARCH64_INSN_IMM_MAX +#define AARCH64_INSN_IMM_MOVK AARCH64_INSN_IMM_16 void *module_alloc(unsigned long size) { @@ -94,28 +98,18 @@ static int reloc_data(enum aarch64_reloc_op op, void *place, u64 val, int len) return 0; } -enum aarch64_imm_type { - INSN_IMM_MOVNZ, - INSN_IMM_MOVK, - INSN_IMM_ADR, - INSN_IMM_26, - INSN_IMM_19, - INSN_IMM_16, - INSN_IMM_14, - INSN_IMM_12, - INSN_IMM_9, -}; - -static u32 encode_insn_immediate(enum aarch64_imm_type type, u32 insn, u64 imm) +static int reloc_insn_movw(enum aarch64_reloc_op op, void *place, u64 val, + int lsb, enum aarch64_insn_imm_type imm_type) { - u32 immlo, immhi, lomask, himask, mask; - int shift; + u64 imm, limit = 0; + s64 sval; + u32 insn = le32_to_cpu(*(u32 *)place); - /* The instruction stream is always little endian. */ - insn = le32_to_cpu(insn); + sval = do_reloc(op, place, val); + sval >>= lsb; + imm = sval & 0xffff; - switch (type) { - case INSN_IMM_MOVNZ: + if (imm_type == AARCH64_INSN_IMM_MOVNZ) { /* * For signed MOVW relocations, we have to manipulate the * instruction encoding depending on whether or not the @@ -134,70 +128,12 @@ static u32 encode_insn_immediate(enum aarch64_imm_type type, u32 insn, u64 imm) */ imm = ~imm; } - case INSN_IMM_MOVK: - mask = BIT(16) - 1; - shift = 5; - break; - case INSN_IMM_ADR: - lomask = 0x3; - himask = 0x7ffff; - immlo = imm & lomask; - imm >>= 2; - immhi = imm & himask; - imm = (immlo << 24) | (immhi); - mask = (lomask << 24) | (himask); - shift = 5; - break; - case INSN_IMM_26: - mask = BIT(26) - 1; - shift = 0; - break; - case INSN_IMM_19: - mask = BIT(19) - 1; - shift = 5; - break; - case INSN_IMM_16: - mask = BIT(16) - 1; - shift = 5; - break; - case INSN_IMM_14: - mask = BIT(14) - 1; - shift = 5; - break; - case INSN_IMM_12: - mask = BIT(12) - 1; - shift = 10; - break; - case INSN_IMM_9: - mask = BIT(9) - 1; - shift = 12; - break; - default: - pr_err("encode_insn_immediate: unknown immediate encoding %d\n", - type); - return 0; + imm_type = AARCH64_INSN_IMM_MOVK; } - /* Update the immediate field. */ - insn &= ~(mask << shift); - insn |= (imm & mask) << shift; - - return cpu_to_le32(insn); -} - -static int reloc_insn_movw(enum aarch64_reloc_op op, void *place, u64 val, - int lsb, enum aarch64_imm_type imm_type) -{ - u64 imm, limit = 0; - s64 sval; - u32 insn = *(u32 *)place; - - sval = do_reloc(op, place, val); - sval >>= lsb; - imm = sval & 0xffff; - /* Update the instruction with the new encoding. */ - *(u32 *)place = encode_insn_immediate(imm_type, insn, imm); + insn = aarch64_insn_encode_immediate(imm_type, insn, imm); + *(u32 *)place = cpu_to_le32(insn); /* Shift out the immediate field. */ sval >>= 16; @@ -206,9 +142,9 @@ static int reloc_insn_movw(enum aarch64_reloc_op op, void *place, u64 val, * For unsigned immediates, the overflow check is straightforward. * For signed immediates, the sign bit is actually the bit past the * most significant bit of the field. - * The INSN_IMM_16 immediate type is unsigned. + * The AARCH64_INSN_IMM_16 immediate type is unsigned. */ - if (imm_type != INSN_IMM_16) { + if (imm_type != AARCH64_INSN_IMM_16) { sval++; limit++; } @@ -221,11 +157,11 @@ static int reloc_insn_movw(enum aarch64_reloc_op op, void *place, u64 val, } static int reloc_insn_imm(enum aarch64_reloc_op op, void *place, u64 val, - int lsb, int len, enum aarch64_imm_type imm_type) + int lsb, int len, enum aarch64_insn_imm_type imm_type) { u64 imm, imm_mask; s64 sval; - u32 insn = *(u32 *)place; + u32 insn = le32_to_cpu(*(u32 *)place); /* Calculate the relocation value. */ sval = do_reloc(op, place, val); @@ -236,7 +172,8 @@ static int reloc_insn_imm(enum aarch64_reloc_op op, void *place, u64 val, imm = sval & imm_mask; /* Update the instruction's immediate field. */ - *(u32 *)place = encode_insn_immediate(imm_type, insn, imm); + insn = aarch64_insn_encode_immediate(imm_type, insn, imm); + *(u32 *)place = cpu_to_le32(insn); /* * Extract the upper value bits (including the sign bit) and @@ -318,125 +255,125 @@ int apply_relocate_add(Elf64_Shdr *sechdrs, overflow_check = false; case R_AARCH64_MOVW_UABS_G0: ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 0, - INSN_IMM_16); + AARCH64_INSN_IMM_16); break; case R_AARCH64_MOVW_UABS_G1_NC: overflow_check = false; case R_AARCH64_MOVW_UABS_G1: ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 16, - INSN_IMM_16); + AARCH64_INSN_IMM_16); break; case R_AARCH64_MOVW_UABS_G2_NC: overflow_check = false; case R_AARCH64_MOVW_UABS_G2: ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 32, - INSN_IMM_16); + AARCH64_INSN_IMM_16); break; case R_AARCH64_MOVW_UABS_G3: /* We're using the top bits so we can't overflow. */ overflow_check = false; ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 48, - INSN_IMM_16); + AARCH64_INSN_IMM_16); break; case R_AARCH64_MOVW_SABS_G0: ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 0, - INSN_IMM_MOVNZ); + AARCH64_INSN_IMM_MOVNZ); break; case R_AARCH64_MOVW_SABS_G1: ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 16, - INSN_IMM_MOVNZ); + AARCH64_INSN_IMM_MOVNZ); break; case R_AARCH64_MOVW_SABS_G2: ovf = reloc_insn_movw(RELOC_OP_ABS, loc, val, 32, - INSN_IMM_MOVNZ); + AARCH64_INSN_IMM_MOVNZ); break; case R_AARCH64_MOVW_PREL_G0_NC: overflow_check = false; ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 0, - INSN_IMM_MOVK); + AARCH64_INSN_IMM_MOVK); break; case R_AARCH64_MOVW_PREL_G0: ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 0, - INSN_IMM_MOVNZ); + AARCH64_INSN_IMM_MOVNZ); break; case R_AARCH64_MOVW_PREL_G1_NC: overflow_check = false; ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 16, - INSN_IMM_MOVK); + AARCH64_INSN_IMM_MOVK); break; case R_AARCH64_MOVW_PREL_G1: ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 16, - INSN_IMM_MOVNZ); + AARCH64_INSN_IMM_MOVNZ); break; case R_AARCH64_MOVW_PREL_G2_NC: overflow_check = false; ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 32, - INSN_IMM_MOVK); + AARCH64_INSN_IMM_MOVK); break; case R_AARCH64_MOVW_PREL_G2: ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 32, - INSN_IMM_MOVNZ); + AARCH64_INSN_IMM_MOVNZ); break; case R_AARCH64_MOVW_PREL_G3: /* We're using the top bits so we can't overflow. */ overflow_check = false; ovf = reloc_insn_movw(RELOC_OP_PREL, loc, val, 48, - INSN_IMM_MOVNZ); + AARCH64_INSN_IMM_MOVNZ); break; /* Immediate instruction relocations. */ case R_AARCH64_LD_PREL_LO19: ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 2, 19, - INSN_IMM_19); + AARCH64_INSN_IMM_19); break; case R_AARCH64_ADR_PREL_LO21: ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 0, 21, - INSN_IMM_ADR); + AARCH64_INSN_IMM_ADR); break; case R_AARCH64_ADR_PREL_PG_HI21_NC: overflow_check = false; case R_AARCH64_ADR_PREL_PG_HI21: ovf = reloc_insn_imm(RELOC_OP_PAGE, loc, val, 12, 21, - INSN_IMM_ADR); + AARCH64_INSN_IMM_ADR); break; case R_AARCH64_ADD_ABS_LO12_NC: case R_AARCH64_LDST8_ABS_LO12_NC: overflow_check = false; ovf = reloc_insn_imm(RELOC_OP_ABS, loc, val, 0, 12, - INSN_IMM_12); + AARCH64_INSN_IMM_12); break; case R_AARCH64_LDST16_ABS_LO12_NC: overflow_check = false; ovf = reloc_insn_imm(RELOC_OP_ABS, loc, val, 1, 11, - INSN_IMM_12); + AARCH64_INSN_IMM_12); break; case R_AARCH64_LDST32_ABS_LO12_NC: overflow_check = false; ovf = reloc_insn_imm(RELOC_OP_ABS, loc, val, 2, 10, - INSN_IMM_12); + AARCH64_INSN_IMM_12); break; case R_AARCH64_LDST64_ABS_LO12_NC: overflow_check = false; ovf = reloc_insn_imm(RELOC_OP_ABS, loc, val, 3, 9, - INSN_IMM_12); + AARCH64_INSN_IMM_12); break; case R_AARCH64_LDST128_ABS_LO12_NC: overflow_check = false; ovf = reloc_insn_imm(RELOC_OP_ABS, loc, val, 4, 8, - INSN_IMM_12); + AARCH64_INSN_IMM_12); break; case R_AARCH64_TSTBR14: ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 2, 14, - INSN_IMM_14); + AARCH64_INSN_IMM_14); break; case R_AARCH64_CONDBR19: ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 2, 19, - INSN_IMM_19); + AARCH64_INSN_IMM_19); break; case R_AARCH64_JUMP26: case R_AARCH64_CALL26: ovf = reloc_insn_imm(RELOC_OP_PREL, loc, val, 2, 26, - INSN_IMM_26); + AARCH64_INSN_IMM_26); break; default: -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v6 4/7] arm64: introduce aarch64_insn_gen_{nop|branch_imm}() helper functions 2013-12-10 16:03 ` Jiang Liu @ 2013-12-10 16:03 ` Jiang Liu -1 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-10 16:03 UTC (permalink / raw) To: linux-arm-kernel Introduce aarch64_insn_gen_{nop|branch_imm}() helper functions, which will be used to implement jump label on ARM64. Signed-off-by: Jiang Liu <liuj97@gmail.com> --- arch/arm64/include/asm/insn.h | 10 ++++++++++ arch/arm64/kernel/insn.c | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h index fb44660..c44ad39 100644 --- a/arch/arm64/include/asm/insn.h +++ b/arch/arm64/include/asm/insn.h @@ -66,6 +66,11 @@ enum aarch64_insn_imm_type { AARCH64_INSN_IMM_MAX }; +enum aarch64_insn_branch_type { + AARCH64_INSN_BRANCH_NOLINK, + AARCH64_INSN_BRANCH_LINK, +}; + #define __AARCH64_INSN_FUNCS(abbr, mask, val) \ static __always_inline bool aarch64_insn_is_##abbr(u32 code) \ { return (code & (mask)) == (val); } \ @@ -89,6 +94,11 @@ int aarch64_insn_write(void *addr, u32 insn); enum aarch64_insn_encoding_class aarch64_get_insn_class(u32 insn); u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type, u32 insn, u64 imm); +u32 aarch64_insn_gen_branch_imm(unsigned long pc, unsigned long addr, + enum aarch64_insn_branch_type type); +u32 aarch64_insn_gen_hint(enum aarch64_insn_hint_op op); +u32 aarch64_insn_gen_nop(void); + bool aarch64_insn_hotpatch_safe(u32 old_insn, u32 new_insn); int aarch64_insn_patch_text_nosync(void *addr, u32 insn); diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c index 03e9a5f..ead3239 100644 --- a/arch/arm64/kernel/insn.c +++ b/arch/arm64/kernel/insn.c @@ -14,6 +14,7 @@ * You should have received a copy of the GNU General Public License * along with this program. If not, see <http://www.gnu.org/licenses/>. */ +#include <linux/bitops.h> #include <linux/compiler.h> #include <linux/kernel.h> #include <linux/smp.h> @@ -260,3 +261,42 @@ u32 __kprobes aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type, return insn; } + +u32 __kprobes aarch64_insn_gen_branch_imm(unsigned long pc, unsigned long addr, + enum aarch64_insn_branch_type type) +{ + u32 insn; + long offset; + + /* + * PC: A 64-bit Program Counter holding the address of the current + * instruction. A64 instructions must be word-aligned. + */ + BUG_ON((pc & 0x3) || (addr & 0x3)); + + /* + * B/BL support [-128M, 128M) offset + * ARM64 virtual address arrangement guarantees all kernel and module + * texts are within +/-128M. + */ + offset = ((long)addr - (long)pc); + BUG_ON(offset < -SZ_128M || offset >= SZ_128M); + + if (type == AARCH64_INSN_BRANCH_LINK) + insn = aarch64_insn_get_bl_value(); + else + insn = aarch64_insn_get_b_value(); + + return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_26, insn, + offset >> 2); +} + +u32 __kprobes aarch64_insn_gen_hint(enum aarch64_insn_hint_op op) +{ + return aarch64_insn_get_hint_value() | op; +} + +u32 __kprobes aarch64_insn_gen_nop(void) +{ + return aarch64_insn_gen_hint(AARCH64_INSN_HINT_NOP); +} -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v6 4/7] arm64: introduce aarch64_insn_gen_{nop|branch_imm}() helper functions @ 2013-12-10 16:03 ` Jiang Liu 0 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-10 16:03 UTC (permalink / raw) To: Steven Rostedt, Catalin Marinas, Will Deacon, Sandeepa Prabhu, AKASHI Takahiro, linux-arm-kernel, linux-kernel Cc: Jiang Liu Introduce aarch64_insn_gen_{nop|branch_imm}() helper functions, which will be used to implement jump label on ARM64. Signed-off-by: Jiang Liu <liuj97@gmail.com> --- arch/arm64/include/asm/insn.h | 10 ++++++++++ arch/arm64/kernel/insn.c | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h index fb44660..c44ad39 100644 --- a/arch/arm64/include/asm/insn.h +++ b/arch/arm64/include/asm/insn.h @@ -66,6 +66,11 @@ enum aarch64_insn_imm_type { AARCH64_INSN_IMM_MAX }; +enum aarch64_insn_branch_type { + AARCH64_INSN_BRANCH_NOLINK, + AARCH64_INSN_BRANCH_LINK, +}; + #define __AARCH64_INSN_FUNCS(abbr, mask, val) \ static __always_inline bool aarch64_insn_is_##abbr(u32 code) \ { return (code & (mask)) == (val); } \ @@ -89,6 +94,11 @@ int aarch64_insn_write(void *addr, u32 insn); enum aarch64_insn_encoding_class aarch64_get_insn_class(u32 insn); u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type, u32 insn, u64 imm); +u32 aarch64_insn_gen_branch_imm(unsigned long pc, unsigned long addr, + enum aarch64_insn_branch_type type); +u32 aarch64_insn_gen_hint(enum aarch64_insn_hint_op op); +u32 aarch64_insn_gen_nop(void); + bool aarch64_insn_hotpatch_safe(u32 old_insn, u32 new_insn); int aarch64_insn_patch_text_nosync(void *addr, u32 insn); diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c index 03e9a5f..ead3239 100644 --- a/arch/arm64/kernel/insn.c +++ b/arch/arm64/kernel/insn.c @@ -14,6 +14,7 @@ * You should have received a copy of the GNU General Public License * along with this program. If not, see <http://www.gnu.org/licenses/>. */ +#include <linux/bitops.h> #include <linux/compiler.h> #include <linux/kernel.h> #include <linux/smp.h> @@ -260,3 +261,42 @@ u32 __kprobes aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type, return insn; } + +u32 __kprobes aarch64_insn_gen_branch_imm(unsigned long pc, unsigned long addr, + enum aarch64_insn_branch_type type) +{ + u32 insn; + long offset; + + /* + * PC: A 64-bit Program Counter holding the address of the current + * instruction. A64 instructions must be word-aligned. + */ + BUG_ON((pc & 0x3) || (addr & 0x3)); + + /* + * B/BL support [-128M, 128M) offset + * ARM64 virtual address arrangement guarantees all kernel and module + * texts are within +/-128M. + */ + offset = ((long)addr - (long)pc); + BUG_ON(offset < -SZ_128M || offset >= SZ_128M); + + if (type == AARCH64_INSN_BRANCH_LINK) + insn = aarch64_insn_get_bl_value(); + else + insn = aarch64_insn_get_b_value(); + + return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_26, insn, + offset >> 2); +} + +u32 __kprobes aarch64_insn_gen_hint(enum aarch64_insn_hint_op op) +{ + return aarch64_insn_get_hint_value() | op; +} + +u32 __kprobes aarch64_insn_gen_nop(void) +{ + return aarch64_insn_gen_hint(AARCH64_INSN_HINT_NOP); +} -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v6 4/7] arm64: introduce aarch64_insn_gen_{nop|branch_imm}() helper functions 2013-12-10 16:03 ` Jiang Liu @ 2013-12-13 15:58 ` Will Deacon -1 siblings, 0 replies; 42+ messages in thread From: Will Deacon @ 2013-12-13 15:58 UTC (permalink / raw) To: linux-arm-kernel On Tue, Dec 10, 2013 at 04:03:50PM +0000, Jiang Liu wrote: > Introduce aarch64_insn_gen_{nop|branch_imm}() helper functions, which > will be used to implement jump label on ARM64. > > Signed-off-by: Jiang Liu <liuj97@gmail.com> Looks good to me: Reviewed-by: Will Deacon <will.deacon@arm.com> Thanks! Will ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v6 4/7] arm64: introduce aarch64_insn_gen_{nop|branch_imm}() helper functions @ 2013-12-13 15:58 ` Will Deacon 0 siblings, 0 replies; 42+ messages in thread From: Will Deacon @ 2013-12-13 15:58 UTC (permalink / raw) To: Jiang Liu Cc: Steven Rostedt, Catalin Marinas, Sandeepa Prabhu, AKASHI Takahiro, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org On Tue, Dec 10, 2013 at 04:03:50PM +0000, Jiang Liu wrote: > Introduce aarch64_insn_gen_{nop|branch_imm}() helper functions, which > will be used to implement jump label on ARM64. > > Signed-off-by: Jiang Liu <liuj97@gmail.com> Looks good to me: Reviewed-by: Will Deacon <will.deacon@arm.com> Thanks! Will ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v6 5/7] arm64, jump label: detect %c support for ARM64 2013-12-10 16:03 ` Jiang Liu @ 2013-12-10 16:03 ` Jiang Liu -1 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-10 16:03 UTC (permalink / raw) To: linux-arm-kernel As commit a9468f30b5eac6 "ARM: 7333/2: jump label: detect %c support for ARM", this patch detects the same thing for ARM64 because some ARM64 GCC versions have the same issue. Some versions of ARM64 GCC which do support asm goto, do not support the %c specifier. Since we need the %c to support jump labels on ARM64, detect that too in the asm goto detection script to avoid build errors with these versions. Acked-by: Will Deacon <will.deacon@arm.com> Signed-off-by: Jiang Liu <liuj97@gmail.com> --- scripts/gcc-goto.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/gcc-goto.sh b/scripts/gcc-goto.sh index a2af2e8..c9469d3 100644 --- a/scripts/gcc-goto.sh +++ b/scripts/gcc-goto.sh @@ -5,7 +5,7 @@ cat << "END" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y" int main(void) { -#ifdef __arm__ +#if defined(__arm__) || defined(__aarch64__) /* * Not related to asm goto, but used by jump label * and broken on some ARM GCC versions (see GCC Bug 48637). -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v6 5/7] arm64, jump label: detect %c support for ARM64 @ 2013-12-10 16:03 ` Jiang Liu 0 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-10 16:03 UTC (permalink / raw) To: Steven Rostedt, Catalin Marinas, Will Deacon, Sandeepa Prabhu, AKASHI Takahiro, linux-kernel Cc: Jiang Liu, linux-arm-kernel As commit a9468f30b5eac6 "ARM: 7333/2: jump label: detect %c support for ARM", this patch detects the same thing for ARM64 because some ARM64 GCC versions have the same issue. Some versions of ARM64 GCC which do support asm goto, do not support the %c specifier. Since we need the %c to support jump labels on ARM64, detect that too in the asm goto detection script to avoid build errors with these versions. Acked-by: Will Deacon <will.deacon@arm.com> Signed-off-by: Jiang Liu <liuj97@gmail.com> --- scripts/gcc-goto.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/gcc-goto.sh b/scripts/gcc-goto.sh index a2af2e8..c9469d3 100644 --- a/scripts/gcc-goto.sh +++ b/scripts/gcc-goto.sh @@ -5,7 +5,7 @@ cat << "END" | $@ -x c - -c -o /dev/null >/dev/null 2>&1 && echo "y" int main(void) { -#ifdef __arm__ +#if defined(__arm__) || defined(__aarch64__) /* * Not related to asm goto, but used by jump label * and broken on some ARM GCC versions (see GCC Bug 48637). -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v6 6/7] arm64, jump label: optimize jump label implementation 2013-12-10 16:03 ` Jiang Liu @ 2013-12-10 16:03 ` Jiang Liu -1 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-10 16:03 UTC (permalink / raw) To: linux-arm-kernel Optimize jump label implementation for ARM64 by dynamically patching kernel text. Signed-off-by: Jiang Liu <liuj97@gmail.com> --- arch/arm64/Kconfig | 1 + arch/arm64/include/asm/jump_label.h | 51 ++++++++++++++++++++++++++++++++ arch/arm64/kernel/Makefile | 1 + arch/arm64/kernel/jump_label.c | 59 +++++++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+) create mode 100644 arch/arm64/include/asm/jump_label.h create mode 100644 arch/arm64/kernel/jump_label.c diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 6d4dd22..5f962e8 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -19,6 +19,7 @@ config ARM64 select GENERIC_SMP_IDLE_THREAD select GENERIC_TIME_VSYSCALL select HARDIRQS_SW_RESEND + select HAVE_ARCH_JUMP_LABEL select HAVE_ARCH_TRACEHOOK select HAVE_DEBUG_BUGVERBOSE select HAVE_DEBUG_KMEMLEAK diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h new file mode 100644 index 0000000..ca95a3a --- /dev/null +++ b/arch/arm64/include/asm/jump_label.h @@ -0,0 +1,51 @@ +/* + * Copyright (C) 2013 Huawei Ltd. + * Author: Jiang Liu <liuj97@gmail.com> + * + * Based on arch/arm/include/asm/jump_label.h + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#ifndef _ASM_ARM64_JUMP_LABEL_H +#define _ASM_ARM64_JUMP_LABEL_H +#include <linux/types.h> + +#ifdef __KERNEL__ + +#define JUMP_LABEL_NOP_SIZE 4 + +static __always_inline bool arch_static_branch(struct static_key *key) +{ + asm goto("1: nop\n\t" + ".pushsection __jump_table, \"aw\"\n\t" + ".align 3\n\t" + ".quad 1b, %l[l_yes], %c0\n\t" + ".popsection\n\t" + : : "i"(key) : : l_yes); + + return false; +l_yes: + return true; +} + +#endif /* __KERNEL__ */ + +typedef u64 jump_label_t; + +struct jump_entry { + jump_label_t code; + jump_label_t target; + jump_label_t key; +}; + +#endif /* _ASM_ARM64_JUMP_LABEL_H */ diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index cd95a25..e155d56 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -18,6 +18,7 @@ arm64-obj-$(CONFIG_SMP) += smp.o smp_spin_table.o arm64-obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o arm64-obj-$(CONFIG_HAVE_HW_BREAKPOINT)+= hw_breakpoint.o arm64-obj-$(CONFIG_EARLY_PRINTK) += early_printk.o +arm64-obj-$(CONFIG_JUMP_LABEL) += jump_label.o obj-y += $(arm64-obj-y) vdso/ obj-m += $(arm64-obj-m) diff --git a/arch/arm64/kernel/jump_label.c b/arch/arm64/kernel/jump_label.c new file mode 100644 index 0000000..b13ef62 --- /dev/null +++ b/arch/arm64/kernel/jump_label.c @@ -0,0 +1,59 @@ +/* + * Copyright (C) 2013 Huawei Ltd. + * Author: Jiang Liu <liuj97@gmail.com> + * + * Based on arch/arm/kernel/jump_label.c + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#include <linux/kernel.h> +#include <linux/jump_label.h> +#include <asm/jump_label.h> +#include <asm/insn.h> + +#ifdef HAVE_JUMP_LABEL + +static void __arch_jump_label_transform(struct jump_entry *entry, + enum jump_label_type type, + bool is_static) +{ + void *addr = (void *)entry->code; + u32 insn; + + if (type == JUMP_LABEL_ENABLE) { + insn = aarch64_insn_gen_branch_imm(entry->code, + entry->target, + AARCH64_INSN_BRANCH_NOLINK); + } else { + insn = aarch64_insn_gen_nop(); + } + + if (is_static) + aarch64_insn_patch_text_nosync(addr, insn); + else + aarch64_insn_patch_text(&addr, &insn, 1); +} + +void arch_jump_label_transform(struct jump_entry *entry, + enum jump_label_type type) +{ + __arch_jump_label_transform(entry, type, false); +} + +void arch_jump_label_transform_static(struct jump_entry *entry, + enum jump_label_type type) +{ + __arch_jump_label_transform(entry, type, true); +} + +#endif /* HAVE_JUMP_LABEL */ -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v6 6/7] arm64, jump label: optimize jump label implementation @ 2013-12-10 16:03 ` Jiang Liu 0 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-10 16:03 UTC (permalink / raw) To: Steven Rostedt, Catalin Marinas, Will Deacon, Sandeepa Prabhu, AKASHI Takahiro, Marc Zyngier, Mark Rutland, Arnd Bergmann, linux-arm-kernel, linux-kernel Cc: Jiang Liu Optimize jump label implementation for ARM64 by dynamically patching kernel text. Signed-off-by: Jiang Liu <liuj97@gmail.com> --- arch/arm64/Kconfig | 1 + arch/arm64/include/asm/jump_label.h | 51 ++++++++++++++++++++++++++++++++ arch/arm64/kernel/Makefile | 1 + arch/arm64/kernel/jump_label.c | 59 +++++++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+) create mode 100644 arch/arm64/include/asm/jump_label.h create mode 100644 arch/arm64/kernel/jump_label.c diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 6d4dd22..5f962e8 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -19,6 +19,7 @@ config ARM64 select GENERIC_SMP_IDLE_THREAD select GENERIC_TIME_VSYSCALL select HARDIRQS_SW_RESEND + select HAVE_ARCH_JUMP_LABEL select HAVE_ARCH_TRACEHOOK select HAVE_DEBUG_BUGVERBOSE select HAVE_DEBUG_KMEMLEAK diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h new file mode 100644 index 0000000..ca95a3a --- /dev/null +++ b/arch/arm64/include/asm/jump_label.h @@ -0,0 +1,51 @@ +/* + * Copyright (C) 2013 Huawei Ltd. + * Author: Jiang Liu <liuj97@gmail.com> + * + * Based on arch/arm/include/asm/jump_label.h + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#ifndef _ASM_ARM64_JUMP_LABEL_H +#define _ASM_ARM64_JUMP_LABEL_H +#include <linux/types.h> + +#ifdef __KERNEL__ + +#define JUMP_LABEL_NOP_SIZE 4 + +static __always_inline bool arch_static_branch(struct static_key *key) +{ + asm goto("1: nop\n\t" + ".pushsection __jump_table, \"aw\"\n\t" + ".align 3\n\t" + ".quad 1b, %l[l_yes], %c0\n\t" + ".popsection\n\t" + : : "i"(key) : : l_yes); + + return false; +l_yes: + return true; +} + +#endif /* __KERNEL__ */ + +typedef u64 jump_label_t; + +struct jump_entry { + jump_label_t code; + jump_label_t target; + jump_label_t key; +}; + +#endif /* _ASM_ARM64_JUMP_LABEL_H */ diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index cd95a25..e155d56 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -18,6 +18,7 @@ arm64-obj-$(CONFIG_SMP) += smp.o smp_spin_table.o arm64-obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o arm64-obj-$(CONFIG_HAVE_HW_BREAKPOINT)+= hw_breakpoint.o arm64-obj-$(CONFIG_EARLY_PRINTK) += early_printk.o +arm64-obj-$(CONFIG_JUMP_LABEL) += jump_label.o obj-y += $(arm64-obj-y) vdso/ obj-m += $(arm64-obj-m) diff --git a/arch/arm64/kernel/jump_label.c b/arch/arm64/kernel/jump_label.c new file mode 100644 index 0000000..b13ef62 --- /dev/null +++ b/arch/arm64/kernel/jump_label.c @@ -0,0 +1,59 @@ +/* + * Copyright (C) 2013 Huawei Ltd. + * Author: Jiang Liu <liuj97@gmail.com> + * + * Based on arch/arm/kernel/jump_label.c + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#include <linux/kernel.h> +#include <linux/jump_label.h> +#include <asm/jump_label.h> +#include <asm/insn.h> + +#ifdef HAVE_JUMP_LABEL + +static void __arch_jump_label_transform(struct jump_entry *entry, + enum jump_label_type type, + bool is_static) +{ + void *addr = (void *)entry->code; + u32 insn; + + if (type == JUMP_LABEL_ENABLE) { + insn = aarch64_insn_gen_branch_imm(entry->code, + entry->target, + AARCH64_INSN_BRANCH_NOLINK); + } else { + insn = aarch64_insn_gen_nop(); + } + + if (is_static) + aarch64_insn_patch_text_nosync(addr, insn); + else + aarch64_insn_patch_text(&addr, &insn, 1); +} + +void arch_jump_label_transform(struct jump_entry *entry, + enum jump_label_type type) +{ + __arch_jump_label_transform(entry, type, false); +} + +void arch_jump_label_transform_static(struct jump_entry *entry, + enum jump_label_type type) +{ + __arch_jump_label_transform(entry, type, true); +} + +#endif /* HAVE_JUMP_LABEL */ -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v6 6/7] arm64, jump label: optimize jump label implementation 2013-12-10 16:03 ` Jiang Liu @ 2013-12-13 15:41 ` Will Deacon -1 siblings, 0 replies; 42+ messages in thread From: Will Deacon @ 2013-12-13 15:41 UTC (permalink / raw) To: linux-arm-kernel Hi Jiang Liu, Thanks for the updated patches! I still have some small comments, but I don't think they will cause you much work. See inline. On Tue, Dec 10, 2013 at 04:03:52PM +0000, Jiang Liu wrote: > Optimize jump label implementation for ARM64 by dynamically patching > kernel text. > > Signed-off-by: Jiang Liu <liuj97@gmail.com> > --- > arch/arm64/Kconfig | 1 + > arch/arm64/include/asm/jump_label.h | 51 ++++++++++++++++++++++++++++++++ > arch/arm64/kernel/Makefile | 1 + > arch/arm64/kernel/jump_label.c | 59 +++++++++++++++++++++++++++++++++++++ > 4 files changed, 112 insertions(+) > create mode 100644 arch/arm64/include/asm/jump_label.h > create mode 100644 arch/arm64/kernel/jump_label.c > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 6d4dd22..5f962e8 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -19,6 +19,7 @@ config ARM64 > select GENERIC_SMP_IDLE_THREAD > select GENERIC_TIME_VSYSCALL > select HARDIRQS_SW_RESEND > + select HAVE_ARCH_JUMP_LABEL > select HAVE_ARCH_TRACEHOOK > select HAVE_DEBUG_BUGVERBOSE > select HAVE_DEBUG_KMEMLEAK > diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h > new file mode 100644 > index 0000000..ca95a3a > --- /dev/null > +++ b/arch/arm64/include/asm/jump_label.h > @@ -0,0 +1,51 @@ > +/* > + * Copyright (C) 2013 Huawei Ltd. > + * Author: Jiang Liu <liuj97@gmail.com> > + * > + * Based on arch/arm/include/asm/jump_label.h > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > +#ifndef _ASM_ARM64_JUMP_LABEL_H > +#define _ASM_ARM64_JUMP_LABEL_H Just use __ASM_JUMP_LABEL_H to keep consistent with out other headers. > +#include <linux/types.h> > + > +#ifdef __KERNEL__ > + > +#define JUMP_LABEL_NOP_SIZE 4 This is the same as AARCH64_INSN_SIZE; just use the latter. > + > +static __always_inline bool arch_static_branch(struct static_key *key) > +{ > + asm goto("1: nop\n\t" > + ".pushsection __jump_table, \"aw\"\n\t" > + ".align 3\n\t" > + ".quad 1b, %l[l_yes], %c0\n\t" > + ".popsection\n\t" > + : : "i"(key) : : l_yes); > + > + return false; > +l_yes: > + return true; > +} > + > +#endif /* __KERNEL__ */ > + > +typedef u64 jump_label_t; > + > +struct jump_entry { > + jump_label_t code; > + jump_label_t target; > + jump_label_t key; > +}; > + > +#endif /* _ASM_ARM64_JUMP_LABEL_H */ > diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile > index cd95a25..e155d56 100644 > --- a/arch/arm64/kernel/Makefile > +++ b/arch/arm64/kernel/Makefile > @@ -18,6 +18,7 @@ arm64-obj-$(CONFIG_SMP) += smp.o smp_spin_table.o > arm64-obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o > arm64-obj-$(CONFIG_HAVE_HW_BREAKPOINT)+= hw_breakpoint.o > arm64-obj-$(CONFIG_EARLY_PRINTK) += early_printk.o > +arm64-obj-$(CONFIG_JUMP_LABEL) += jump_label.o > > obj-y += $(arm64-obj-y) vdso/ > obj-m += $(arm64-obj-m) > diff --git a/arch/arm64/kernel/jump_label.c b/arch/arm64/kernel/jump_label.c > new file mode 100644 > index 0000000..b13ef62 > --- /dev/null > +++ b/arch/arm64/kernel/jump_label.c > @@ -0,0 +1,59 @@ > +/* > + * Copyright (C) 2013 Huawei Ltd. > + * Author: Jiang Liu <liuj97@gmail.com> > + * > + * Based on arch/arm/kernel/jump_label.c > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > +#include <linux/kernel.h> > +#include <linux/jump_label.h> > +#include <asm/jump_label.h> > +#include <asm/insn.h> > + > +#ifdef HAVE_JUMP_LABEL Slightly worrying... this should be CONFIG_HAVE_JUMP_LABEL, right? How did you manage to test this code? Will ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v6 6/7] arm64, jump label: optimize jump label implementation @ 2013-12-13 15:41 ` Will Deacon 0 siblings, 0 replies; 42+ messages in thread From: Will Deacon @ 2013-12-13 15:41 UTC (permalink / raw) To: Jiang Liu Cc: Steven Rostedt, Catalin Marinas, Sandeepa Prabhu, AKASHI Takahiro, Marc Zyngier, Mark Rutland, Arnd Bergmann, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Hi Jiang Liu, Thanks for the updated patches! I still have some small comments, but I don't think they will cause you much work. See inline. On Tue, Dec 10, 2013 at 04:03:52PM +0000, Jiang Liu wrote: > Optimize jump label implementation for ARM64 by dynamically patching > kernel text. > > Signed-off-by: Jiang Liu <liuj97@gmail.com> > --- > arch/arm64/Kconfig | 1 + > arch/arm64/include/asm/jump_label.h | 51 ++++++++++++++++++++++++++++++++ > arch/arm64/kernel/Makefile | 1 + > arch/arm64/kernel/jump_label.c | 59 +++++++++++++++++++++++++++++++++++++ > 4 files changed, 112 insertions(+) > create mode 100644 arch/arm64/include/asm/jump_label.h > create mode 100644 arch/arm64/kernel/jump_label.c > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 6d4dd22..5f962e8 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -19,6 +19,7 @@ config ARM64 > select GENERIC_SMP_IDLE_THREAD > select GENERIC_TIME_VSYSCALL > select HARDIRQS_SW_RESEND > + select HAVE_ARCH_JUMP_LABEL > select HAVE_ARCH_TRACEHOOK > select HAVE_DEBUG_BUGVERBOSE > select HAVE_DEBUG_KMEMLEAK > diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h > new file mode 100644 > index 0000000..ca95a3a > --- /dev/null > +++ b/arch/arm64/include/asm/jump_label.h > @@ -0,0 +1,51 @@ > +/* > + * Copyright (C) 2013 Huawei Ltd. > + * Author: Jiang Liu <liuj97@gmail.com> > + * > + * Based on arch/arm/include/asm/jump_label.h > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > +#ifndef _ASM_ARM64_JUMP_LABEL_H > +#define _ASM_ARM64_JUMP_LABEL_H Just use __ASM_JUMP_LABEL_H to keep consistent with out other headers. > +#include <linux/types.h> > + > +#ifdef __KERNEL__ > + > +#define JUMP_LABEL_NOP_SIZE 4 This is the same as AARCH64_INSN_SIZE; just use the latter. > + > +static __always_inline bool arch_static_branch(struct static_key *key) > +{ > + asm goto("1: nop\n\t" > + ".pushsection __jump_table, \"aw\"\n\t" > + ".align 3\n\t" > + ".quad 1b, %l[l_yes], %c0\n\t" > + ".popsection\n\t" > + : : "i"(key) : : l_yes); > + > + return false; > +l_yes: > + return true; > +} > + > +#endif /* __KERNEL__ */ > + > +typedef u64 jump_label_t; > + > +struct jump_entry { > + jump_label_t code; > + jump_label_t target; > + jump_label_t key; > +}; > + > +#endif /* _ASM_ARM64_JUMP_LABEL_H */ > diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile > index cd95a25..e155d56 100644 > --- a/arch/arm64/kernel/Makefile > +++ b/arch/arm64/kernel/Makefile > @@ -18,6 +18,7 @@ arm64-obj-$(CONFIG_SMP) += smp.o smp_spin_table.o > arm64-obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o > arm64-obj-$(CONFIG_HAVE_HW_BREAKPOINT)+= hw_breakpoint.o > arm64-obj-$(CONFIG_EARLY_PRINTK) += early_printk.o > +arm64-obj-$(CONFIG_JUMP_LABEL) += jump_label.o > > obj-y += $(arm64-obj-y) vdso/ > obj-m += $(arm64-obj-m) > diff --git a/arch/arm64/kernel/jump_label.c b/arch/arm64/kernel/jump_label.c > new file mode 100644 > index 0000000..b13ef62 > --- /dev/null > +++ b/arch/arm64/kernel/jump_label.c > @@ -0,0 +1,59 @@ > +/* > + * Copyright (C) 2013 Huawei Ltd. > + * Author: Jiang Liu <liuj97@gmail.com> > + * > + * Based on arch/arm/kernel/jump_label.c > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > +#include <linux/kernel.h> > +#include <linux/jump_label.h> > +#include <asm/jump_label.h> > +#include <asm/insn.h> > + > +#ifdef HAVE_JUMP_LABEL Slightly worrying... this should be CONFIG_HAVE_JUMP_LABEL, right? How did you manage to test this code? Will ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v6 6/7] arm64, jump label: optimize jump label implementation 2013-12-13 15:41 ` Will Deacon @ 2013-12-13 15:50 ` Steven Rostedt -1 siblings, 0 replies; 42+ messages in thread From: Steven Rostedt @ 2013-12-13 15:50 UTC (permalink / raw) To: linux-arm-kernel On Fri, 13 Dec 2013 15:41:04 +0000 Will Deacon <will.deacon@arm.com> wrote: > > +#include <linux/kernel.h> > > +#include <linux/jump_label.h> > > +#include <asm/jump_label.h> > > +#include <asm/insn.h> > > + > > +#ifdef HAVE_JUMP_LABEL > > Slightly worrying... this should be CONFIG_HAVE_JUMP_LABEL, right? How did > you manage to test this code? In <linux/jump_label.h> we have: #if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL) [...] # include <asm/jump_label.h> # define HAVE_JUMP_LABEL #endif /* CC_HAVE_ASM_GOTO && CONFIG_JUMP_LABEL */ -- Steve ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v6 6/7] arm64, jump label: optimize jump label implementation @ 2013-12-13 15:50 ` Steven Rostedt 0 siblings, 0 replies; 42+ messages in thread From: Steven Rostedt @ 2013-12-13 15:50 UTC (permalink / raw) To: Will Deacon Cc: Jiang Liu, Catalin Marinas, Sandeepa Prabhu, AKASHI Takahiro, Marc Zyngier, Mark Rutland, Arnd Bergmann, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org On Fri, 13 Dec 2013 15:41:04 +0000 Will Deacon <will.deacon@arm.com> wrote: > > +#include <linux/kernel.h> > > +#include <linux/jump_label.h> > > +#include <asm/jump_label.h> > > +#include <asm/insn.h> > > + > > +#ifdef HAVE_JUMP_LABEL > > Slightly worrying... this should be CONFIG_HAVE_JUMP_LABEL, right? How did > you manage to test this code? In <linux/jump_label.h> we have: #if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL) [...] # include <asm/jump_label.h> # define HAVE_JUMP_LABEL #endif /* CC_HAVE_ASM_GOTO && CONFIG_JUMP_LABEL */ -- Steve ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v6 6/7] arm64, jump label: optimize jump label implementation 2013-12-13 15:50 ` Steven Rostedt @ 2013-12-13 15:52 ` Steven Rostedt -1 siblings, 0 replies; 42+ messages in thread From: Steven Rostedt @ 2013-12-13 15:52 UTC (permalink / raw) To: linux-arm-kernel On Fri, 13 Dec 2013 10:50:23 -0500 Steven Rostedt <rostedt@goodmis.org> wrote: > On Fri, 13 Dec 2013 15:41:04 +0000 > Will Deacon <will.deacon@arm.com> wrote: > > > > +#include <linux/kernel.h> > > > +#include <linux/jump_label.h> > > > +#include <asm/jump_label.h> > > > +#include <asm/insn.h> > > > + > > > +#ifdef HAVE_JUMP_LABEL > > > > Slightly worrying... this should be CONFIG_HAVE_JUMP_LABEL, right? How did > > you manage to test this code? > > In <linux/jump_label.h> we have: > > #if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL) > > [...] > > # include <asm/jump_label.h> This also makes the include of <asm/jump_label.h> in the C file redundant. -- Steve > # define HAVE_JUMP_LABEL > #endif /* CC_HAVE_ASM_GOTO && CONFIG_JUMP_LABEL */ > > > -- Steve ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v6 6/7] arm64, jump label: optimize jump label implementation @ 2013-12-13 15:52 ` Steven Rostedt 0 siblings, 0 replies; 42+ messages in thread From: Steven Rostedt @ 2013-12-13 15:52 UTC (permalink / raw) To: Steven Rostedt Cc: Will Deacon, Jiang Liu, Catalin Marinas, Sandeepa Prabhu, AKASHI Takahiro, Marc Zyngier, Mark Rutland, Arnd Bergmann, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org On Fri, 13 Dec 2013 10:50:23 -0500 Steven Rostedt <rostedt@goodmis.org> wrote: > On Fri, 13 Dec 2013 15:41:04 +0000 > Will Deacon <will.deacon@arm.com> wrote: > > > > +#include <linux/kernel.h> > > > +#include <linux/jump_label.h> > > > +#include <asm/jump_label.h> > > > +#include <asm/insn.h> > > > + > > > +#ifdef HAVE_JUMP_LABEL > > > > Slightly worrying... this should be CONFIG_HAVE_JUMP_LABEL, right? How did > > you manage to test this code? > > In <linux/jump_label.h> we have: > > #if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL) > > [...] > > # include <asm/jump_label.h> This also makes the include of <asm/jump_label.h> in the C file redundant. -- Steve > # define HAVE_JUMP_LABEL > #endif /* CC_HAVE_ASM_GOTO && CONFIG_JUMP_LABEL */ > > > -- Steve ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v6 6/7] arm64, jump label: optimize jump label implementation 2013-12-13 15:52 ` Steven Rostedt @ 2013-12-15 11:07 ` Jiang Liu -1 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-15 11:07 UTC (permalink / raw) To: linux-arm-kernel On 12/13/2013 11:52 PM, Steven Rostedt wrote: > On Fri, 13 Dec 2013 10:50:23 -0500 > Steven Rostedt <rostedt@goodmis.org> wrote: > >> On Fri, 13 Dec 2013 15:41:04 +0000 >> Will Deacon <will.deacon@arm.com> wrote: >> >>>> +#include <linux/kernel.h> >>>> +#include <linux/jump_label.h> >>>> +#include <asm/jump_label.h> >>>> +#include <asm/insn.h> >>>> + >>>> +#ifdef HAVE_JUMP_LABEL >>> >>> Slightly worrying... this should be CONFIG_HAVE_JUMP_LABEL, right? How did >>> you manage to test this code? >> >> In <linux/jump_label.h> we have: >> >> #if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL) >> >> [...] >> >> # include <asm/jump_label.h> > > This also makes the include of <asm/jump_label.h> in the C file > redundant. > > -- Steve Thanks, Steve. Will remove the redundant include header file. > >> # define HAVE_JUMP_LABEL >> #endif /* CC_HAVE_ASM_GOTO && CONFIG_JUMP_LABEL */ >> >> >> -- Steve > ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v6 6/7] arm64, jump label: optimize jump label implementation @ 2013-12-15 11:07 ` Jiang Liu 0 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-15 11:07 UTC (permalink / raw) To: Steven Rostedt Cc: Will Deacon, Catalin Marinas, Sandeepa Prabhu, AKASHI Takahiro, Marc Zyngier, Mark Rutland, Arnd Bergmann, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org On 12/13/2013 11:52 PM, Steven Rostedt wrote: > On Fri, 13 Dec 2013 10:50:23 -0500 > Steven Rostedt <rostedt@goodmis.org> wrote: > >> On Fri, 13 Dec 2013 15:41:04 +0000 >> Will Deacon <will.deacon@arm.com> wrote: >> >>>> +#include <linux/kernel.h> >>>> +#include <linux/jump_label.h> >>>> +#include <asm/jump_label.h> >>>> +#include <asm/insn.h> >>>> + >>>> +#ifdef HAVE_JUMP_LABEL >>> >>> Slightly worrying... this should be CONFIG_HAVE_JUMP_LABEL, right? How did >>> you manage to test this code? >> >> In <linux/jump_label.h> we have: >> >> #if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL) >> >> [...] >> >> # include <asm/jump_label.h> > > This also makes the include of <asm/jump_label.h> in the C file > redundant. > > -- Steve Thanks, Steve. Will remove the redundant include header file. > >> # define HAVE_JUMP_LABEL >> #endif /* CC_HAVE_ASM_GOTO && CONFIG_JUMP_LABEL */ >> >> >> -- Steve > ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v6 6/7] arm64, jump label: optimize jump label implementation 2013-12-13 15:50 ` Steven Rostedt @ 2013-12-13 15:55 ` Will Deacon -1 siblings, 0 replies; 42+ messages in thread From: Will Deacon @ 2013-12-13 15:55 UTC (permalink / raw) To: linux-arm-kernel On Fri, Dec 13, 2013 at 03:50:23PM +0000, Steven Rostedt wrote: > On Fri, 13 Dec 2013 15:41:04 +0000 > Will Deacon <will.deacon@arm.com> wrote: > > > > +#include <linux/kernel.h> > > > +#include <linux/jump_label.h> > > > +#include <asm/jump_label.h> > > > +#include <asm/insn.h> > > > + > > > +#ifdef HAVE_JUMP_LABEL > > > > Slightly worrying... this should be CONFIG_HAVE_JUMP_LABEL, right? How did > > you manage to test this code? > > In <linux/jump_label.h> we have: > > #if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL) > > [...] > > # include <asm/jump_label.h> > # define HAVE_JUMP_LABEL > #endif /* CC_HAVE_ASM_GOTO && CONFIG_JUMP_LABEL */ > Phew, that explains things! Cheers, Steve. Will ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v6 6/7] arm64, jump label: optimize jump label implementation @ 2013-12-13 15:55 ` Will Deacon 0 siblings, 0 replies; 42+ messages in thread From: Will Deacon @ 2013-12-13 15:55 UTC (permalink / raw) To: Steven Rostedt Cc: Jiang Liu, Catalin Marinas, Sandeepa Prabhu, AKASHI Takahiro, Marc Zyngier, Mark Rutland, Arnd Bergmann, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org On Fri, Dec 13, 2013 at 03:50:23PM +0000, Steven Rostedt wrote: > On Fri, 13 Dec 2013 15:41:04 +0000 > Will Deacon <will.deacon@arm.com> wrote: > > > > +#include <linux/kernel.h> > > > +#include <linux/jump_label.h> > > > +#include <asm/jump_label.h> > > > +#include <asm/insn.h> > > > + > > > +#ifdef HAVE_JUMP_LABEL > > > > Slightly worrying... this should be CONFIG_HAVE_JUMP_LABEL, right? How did > > you manage to test this code? > > In <linux/jump_label.h> we have: > > #if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL) > > [...] > > # include <asm/jump_label.h> > # define HAVE_JUMP_LABEL > #endif /* CC_HAVE_ASM_GOTO && CONFIG_JUMP_LABEL */ > Phew, that explains things! Cheers, Steve. Will ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v6 6/7] arm64, jump label: optimize jump label implementation 2013-12-13 15:41 ` Will Deacon @ 2013-12-15 11:07 ` Jiang Liu -1 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-15 11:07 UTC (permalink / raw) To: linux-arm-kernel Hi Will, Thanks for review. On 12/13/2013 11:41 PM, Will Deacon wrote: > Hi Jiang Liu, > > Thanks for the updated patches! I still have some small comments, but I > don't think they will cause you much work. See inline. > > On Tue, Dec 10, 2013 at 04:03:52PM +0000, Jiang Liu wrote: >> Optimize jump label implementation for ARM64 by dynamically patching >> kernel text. >> >> Signed-off-by: Jiang Liu <liuj97@gmail.com> ... >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program. If not, see <http://www.gnu.org/licenses/>. >> + */ >> +#ifndef _ASM_ARM64_JUMP_LABEL_H >> +#define _ASM_ARM64_JUMP_LABEL_H > > Just use __ASM_JUMP_LABEL_H to keep consistent with out other headers. Sure, will make this change. > >> +#include <linux/types.h> >> + >> +#ifdef __KERNEL__ >> + >> +#define JUMP_LABEL_NOP_SIZE 4 > > This is the same as AARCH64_INSN_SIZE; just use the latter. Yes, it's the same as AARCH64_INSN_SIZE but JUMP_LABEL_NOP_SIZE is needed by the jump label core. So I directly define JUMP_LABEL_NOP_SIZE as 4 instead of AARCH64_INSN_SIZE to avoid pulling asm/insn.h into asm/jump_label.h. Thanks! Gerry > >> + >> +static __always_inline bool arch_static_branch(struct static_key *key) >> +{ >> + asm goto("1: nop\n\t" >> + ".pushsection __jump_table, \"aw\"\n\t" >> + ".align 3\n\t" >> + ".quad 1b, %l[l_yes], %c0\n\t" >> + ".popsection\n\t" >> + : : "i"(key) : : l_yes); >> + >> + return false; >> +l_yes: >> + return true; >> +} >> + >> +#endif /* __KERNEL__ */ >> + >> +typedef u64 jump_label_t; >> + >> +struct jump_entry { >> + jump_label_t code; >> + jump_label_t target; >> + jump_label_t key; >> +}; >> + >> +#endif /* _ASM_ARM64_JUMP_LABEL_H */ >> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile >> index cd95a25..e155d56 100644 >> --- a/arch/arm64/kernel/Makefile >> +++ b/arch/arm64/kernel/Makefile >> @@ -18,6 +18,7 @@ arm64-obj-$(CONFIG_SMP) += smp.o smp_spin_table.o >> arm64-obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o >> arm64-obj-$(CONFIG_HAVE_HW_BREAKPOINT)+= hw_breakpoint.o >> arm64-obj-$(CONFIG_EARLY_PRINTK) += early_printk.o >> +arm64-obj-$(CONFIG_JUMP_LABEL) += jump_label.o >> >> obj-y += $(arm64-obj-y) vdso/ >> obj-m += $(arm64-obj-m) >> diff --git a/arch/arm64/kernel/jump_label.c b/arch/arm64/kernel/jump_label.c >> new file mode 100644 >> index 0000000..b13ef62 >> --- /dev/null >> +++ b/arch/arm64/kernel/jump_label.c >> @@ -0,0 +1,59 @@ >> +/* >> + * Copyright (C) 2013 Huawei Ltd. >> + * Author: Jiang Liu <liuj97@gmail.com> >> + * >> + * Based on arch/arm/kernel/jump_label.c >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program. If not, see <http://www.gnu.org/licenses/>. >> + */ >> +#include <linux/kernel.h> >> +#include <linux/jump_label.h> >> +#include <asm/jump_label.h> >> +#include <asm/insn.h> >> + >> +#ifdef HAVE_JUMP_LABEL > > Slightly worrying... this should be CONFIG_HAVE_JUMP_LABEL, right? How did > you manage to test this code? > > Will > ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v6 6/7] arm64, jump label: optimize jump label implementation @ 2013-12-15 11:07 ` Jiang Liu 0 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-15 11:07 UTC (permalink / raw) To: Will Deacon Cc: Steven Rostedt, Catalin Marinas, Sandeepa Prabhu, AKASHI Takahiro, Marc Zyngier, Mark Rutland, Arnd Bergmann, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Hi Will, Thanks for review. On 12/13/2013 11:41 PM, Will Deacon wrote: > Hi Jiang Liu, > > Thanks for the updated patches! I still have some small comments, but I > don't think they will cause you much work. See inline. > > On Tue, Dec 10, 2013 at 04:03:52PM +0000, Jiang Liu wrote: >> Optimize jump label implementation for ARM64 by dynamically patching >> kernel text. >> >> Signed-off-by: Jiang Liu <liuj97@gmail.com> ... >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program. If not, see <http://www.gnu.org/licenses/>. >> + */ >> +#ifndef _ASM_ARM64_JUMP_LABEL_H >> +#define _ASM_ARM64_JUMP_LABEL_H > > Just use __ASM_JUMP_LABEL_H to keep consistent with out other headers. Sure, will make this change. > >> +#include <linux/types.h> >> + >> +#ifdef __KERNEL__ >> + >> +#define JUMP_LABEL_NOP_SIZE 4 > > This is the same as AARCH64_INSN_SIZE; just use the latter. Yes, it's the same as AARCH64_INSN_SIZE but JUMP_LABEL_NOP_SIZE is needed by the jump label core. So I directly define JUMP_LABEL_NOP_SIZE as 4 instead of AARCH64_INSN_SIZE to avoid pulling asm/insn.h into asm/jump_label.h. Thanks! Gerry > >> + >> +static __always_inline bool arch_static_branch(struct static_key *key) >> +{ >> + asm goto("1: nop\n\t" >> + ".pushsection __jump_table, \"aw\"\n\t" >> + ".align 3\n\t" >> + ".quad 1b, %l[l_yes], %c0\n\t" >> + ".popsection\n\t" >> + : : "i"(key) : : l_yes); >> + >> + return false; >> +l_yes: >> + return true; >> +} >> + >> +#endif /* __KERNEL__ */ >> + >> +typedef u64 jump_label_t; >> + >> +struct jump_entry { >> + jump_label_t code; >> + jump_label_t target; >> + jump_label_t key; >> +}; >> + >> +#endif /* _ASM_ARM64_JUMP_LABEL_H */ >> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile >> index cd95a25..e155d56 100644 >> --- a/arch/arm64/kernel/Makefile >> +++ b/arch/arm64/kernel/Makefile >> @@ -18,6 +18,7 @@ arm64-obj-$(CONFIG_SMP) += smp.o smp_spin_table.o >> arm64-obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o >> arm64-obj-$(CONFIG_HAVE_HW_BREAKPOINT)+= hw_breakpoint.o >> arm64-obj-$(CONFIG_EARLY_PRINTK) += early_printk.o >> +arm64-obj-$(CONFIG_JUMP_LABEL) += jump_label.o >> >> obj-y += $(arm64-obj-y) vdso/ >> obj-m += $(arm64-obj-m) >> diff --git a/arch/arm64/kernel/jump_label.c b/arch/arm64/kernel/jump_label.c >> new file mode 100644 >> index 0000000..b13ef62 >> --- /dev/null >> +++ b/arch/arm64/kernel/jump_label.c >> @@ -0,0 +1,59 @@ >> +/* >> + * Copyright (C) 2013 Huawei Ltd. >> + * Author: Jiang Liu <liuj97@gmail.com> >> + * >> + * Based on arch/arm/kernel/jump_label.c >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program. If not, see <http://www.gnu.org/licenses/>. >> + */ >> +#include <linux/kernel.h> >> +#include <linux/jump_label.h> >> +#include <asm/jump_label.h> >> +#include <asm/insn.h> >> + >> +#ifdef HAVE_JUMP_LABEL > > Slightly worrying... this should be CONFIG_HAVE_JUMP_LABEL, right? How did > you manage to test this code? > > Will > ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v6 7/7] jump_label: use defined macros instead of hard-coding for better readability 2013-12-10 16:03 ` Jiang Liu @ 2013-12-10 16:03 ` Jiang Liu -1 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-10 16:03 UTC (permalink / raw) To: linux-arm-kernel Use macro JUMP_LABEL_TRUE_BRANCH instead of hard-coding for better readability. Signed-off-by: Jiang Liu <liuj97@gmail.com> --- include/linux/jump_label.h | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h index 3999977..192ccba 100644 --- a/include/linux/jump_label.h +++ b/include/linux/jump_label.h @@ -81,18 +81,21 @@ struct module; #include <linux/atomic.h> #ifdef HAVE_JUMP_LABEL -#define JUMP_LABEL_TRUE_BRANCH 1UL +#define JUMP_LABEL_TYPE_FALSE_BRANCH 0UL +#define JUMP_LABEL_TYPE_TRUE_BRANCH 1UL +#define JUMP_LABEL_TYPE_MASK 1UL static inline struct jump_entry *jump_label_get_entries(struct static_key *key) { return (struct jump_entry *)((unsigned long)key->entries - & ~JUMP_LABEL_TRUE_BRANCH); + & ~JUMP_LABEL_TYPE_MASK); } static inline bool jump_label_get_branch_default(struct static_key *key) { - if ((unsigned long)key->entries & JUMP_LABEL_TRUE_BRANCH) + if (((unsigned long)key->entries & JUMP_LABEL_TYPE_MASK) == + JUMP_LABEL_TYPE_TRUE_BRANCH) return true; return false; } @@ -123,9 +126,11 @@ extern void static_key_slow_dec(struct static_key *key); extern void jump_label_apply_nops(struct module *mod); #define STATIC_KEY_INIT_TRUE ((struct static_key) \ - { .enabled = ATOMIC_INIT(1), .entries = (void *)1 }) + { .enabled = ATOMIC_INIT(1), \ + .entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH }) #define STATIC_KEY_INIT_FALSE ((struct static_key) \ - { .enabled = ATOMIC_INIT(0), .entries = (void *)0 }) + { .enabled = ATOMIC_INIT(0), \ + .entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH }) #else /* !HAVE_JUMP_LABEL */ -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v6 7/7] jump_label: use defined macros instead of hard-coding for better readability @ 2013-12-10 16:03 ` Jiang Liu 0 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-10 16:03 UTC (permalink / raw) To: Steven Rostedt, Catalin Marinas, Will Deacon, Sandeepa Prabhu, AKASHI Takahiro, Andrew Morton, Hannes Frederic Sowa, Andrew Jones, David S. Miller, Konrad Rzeszutek Wilk, linux-kernel Cc: Jiang Liu, linux-arm-kernel Use macro JUMP_LABEL_TRUE_BRANCH instead of hard-coding for better readability. Signed-off-by: Jiang Liu <liuj97@gmail.com> --- include/linux/jump_label.h | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h index 3999977..192ccba 100644 --- a/include/linux/jump_label.h +++ b/include/linux/jump_label.h @@ -81,18 +81,21 @@ struct module; #include <linux/atomic.h> #ifdef HAVE_JUMP_LABEL -#define JUMP_LABEL_TRUE_BRANCH 1UL +#define JUMP_LABEL_TYPE_FALSE_BRANCH 0UL +#define JUMP_LABEL_TYPE_TRUE_BRANCH 1UL +#define JUMP_LABEL_TYPE_MASK 1UL static inline struct jump_entry *jump_label_get_entries(struct static_key *key) { return (struct jump_entry *)((unsigned long)key->entries - & ~JUMP_LABEL_TRUE_BRANCH); + & ~JUMP_LABEL_TYPE_MASK); } static inline bool jump_label_get_branch_default(struct static_key *key) { - if ((unsigned long)key->entries & JUMP_LABEL_TRUE_BRANCH) + if (((unsigned long)key->entries & JUMP_LABEL_TYPE_MASK) == + JUMP_LABEL_TYPE_TRUE_BRANCH) return true; return false; } @@ -123,9 +126,11 @@ extern void static_key_slow_dec(struct static_key *key); extern void jump_label_apply_nops(struct module *mod); #define STATIC_KEY_INIT_TRUE ((struct static_key) \ - { .enabled = ATOMIC_INIT(1), .entries = (void *)1 }) + { .enabled = ATOMIC_INIT(1), \ + .entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH }) #define STATIC_KEY_INIT_FALSE ((struct static_key) \ - { .enabled = ATOMIC_INIT(0), .entries = (void *)0 }) + { .enabled = ATOMIC_INIT(0), \ + .entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH }) #else /* !HAVE_JUMP_LABEL */ -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v6 7/7] jump_label: use defined macros instead of hard-coding for better readability 2013-12-10 16:03 ` Jiang Liu @ 2013-12-13 16:00 ` Will Deacon -1 siblings, 0 replies; 42+ messages in thread From: Will Deacon @ 2013-12-13 16:00 UTC (permalink / raw) To: linux-arm-kernel On Tue, Dec 10, 2013 at 04:03:53PM +0000, Jiang Liu wrote: > Use macro JUMP_LABEL_TRUE_BRANCH instead of hard-coding for better > readability. > > Signed-off-by: Jiang Liu <liuj97@gmail.com> > --- > include/linux/jump_label.h | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) Whilst this looks alright to me, I think we probably need an ack from Steve in order to take changes to the core code. Will > diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h > index 3999977..192ccba 100644 > --- a/include/linux/jump_label.h > +++ b/include/linux/jump_label.h > @@ -81,18 +81,21 @@ struct module; > #include <linux/atomic.h> > #ifdef HAVE_JUMP_LABEL > > -#define JUMP_LABEL_TRUE_BRANCH 1UL > +#define JUMP_LABEL_TYPE_FALSE_BRANCH 0UL > +#define JUMP_LABEL_TYPE_TRUE_BRANCH 1UL > +#define JUMP_LABEL_TYPE_MASK 1UL > > static > inline struct jump_entry *jump_label_get_entries(struct static_key *key) > { > return (struct jump_entry *)((unsigned long)key->entries > - & ~JUMP_LABEL_TRUE_BRANCH); > + & ~JUMP_LABEL_TYPE_MASK); > } > > static inline bool jump_label_get_branch_default(struct static_key *key) > { > - if ((unsigned long)key->entries & JUMP_LABEL_TRUE_BRANCH) > + if (((unsigned long)key->entries & JUMP_LABEL_TYPE_MASK) == > + JUMP_LABEL_TYPE_TRUE_BRANCH) > return true; > return false; > } > @@ -123,9 +126,11 @@ extern void static_key_slow_dec(struct static_key *key); > extern void jump_label_apply_nops(struct module *mod); > > #define STATIC_KEY_INIT_TRUE ((struct static_key) \ > - { .enabled = ATOMIC_INIT(1), .entries = (void *)1 }) > + { .enabled = ATOMIC_INIT(1), \ > + .entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH }) > #define STATIC_KEY_INIT_FALSE ((struct static_key) \ > - { .enabled = ATOMIC_INIT(0), .entries = (void *)0 }) > + { .enabled = ATOMIC_INIT(0), \ > + .entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH }) > > #else /* !HAVE_JUMP_LABEL */ > > -- > 1.8.1.2 > > ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v6 7/7] jump_label: use defined macros instead of hard-coding for better readability @ 2013-12-13 16:00 ` Will Deacon 0 siblings, 0 replies; 42+ messages in thread From: Will Deacon @ 2013-12-13 16:00 UTC (permalink / raw) To: Jiang Liu Cc: Steven Rostedt, Catalin Marinas, Sandeepa Prabhu, AKASHI Takahiro, Andrew Morton, Hannes Frederic Sowa, Andrew Jones, David S. Miller, Konrad Rzeszutek Wilk, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org On Tue, Dec 10, 2013 at 04:03:53PM +0000, Jiang Liu wrote: > Use macro JUMP_LABEL_TRUE_BRANCH instead of hard-coding for better > readability. > > Signed-off-by: Jiang Liu <liuj97@gmail.com> > --- > include/linux/jump_label.h | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) Whilst this looks alright to me, I think we probably need an ack from Steve in order to take changes to the core code. Will > diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h > index 3999977..192ccba 100644 > --- a/include/linux/jump_label.h > +++ b/include/linux/jump_label.h > @@ -81,18 +81,21 @@ struct module; > #include <linux/atomic.h> > #ifdef HAVE_JUMP_LABEL > > -#define JUMP_LABEL_TRUE_BRANCH 1UL > +#define JUMP_LABEL_TYPE_FALSE_BRANCH 0UL > +#define JUMP_LABEL_TYPE_TRUE_BRANCH 1UL > +#define JUMP_LABEL_TYPE_MASK 1UL > > static > inline struct jump_entry *jump_label_get_entries(struct static_key *key) > { > return (struct jump_entry *)((unsigned long)key->entries > - & ~JUMP_LABEL_TRUE_BRANCH); > + & ~JUMP_LABEL_TYPE_MASK); > } > > static inline bool jump_label_get_branch_default(struct static_key *key) > { > - if ((unsigned long)key->entries & JUMP_LABEL_TRUE_BRANCH) > + if (((unsigned long)key->entries & JUMP_LABEL_TYPE_MASK) == > + JUMP_LABEL_TYPE_TRUE_BRANCH) > return true; > return false; > } > @@ -123,9 +126,11 @@ extern void static_key_slow_dec(struct static_key *key); > extern void jump_label_apply_nops(struct module *mod); > > #define STATIC_KEY_INIT_TRUE ((struct static_key) \ > - { .enabled = ATOMIC_INIT(1), .entries = (void *)1 }) > + { .enabled = ATOMIC_INIT(1), \ > + .entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH }) > #define STATIC_KEY_INIT_FALSE ((struct static_key) \ > - { .enabled = ATOMIC_INIT(0), .entries = (void *)0 }) > + { .enabled = ATOMIC_INIT(0), \ > + .entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH }) > > #else /* !HAVE_JUMP_LABEL */ > > -- > 1.8.1.2 > > ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v6 7/7] jump_label: use defined macros instead of hard-coding for better readability 2013-12-13 16:00 ` Will Deacon @ 2013-12-13 16:11 ` Steven Rostedt -1 siblings, 0 replies; 42+ messages in thread From: Steven Rostedt @ 2013-12-13 16:11 UTC (permalink / raw) To: linux-arm-kernel [ added Jason Baron too (the original author) ] On Fri, 13 Dec 2013 16:00:41 +0000 Will Deacon <will.deacon@arm.com> wrote: > On Tue, Dec 10, 2013 at 04:03:53PM +0000, Jiang Liu wrote: > > Use macro JUMP_LABEL_TRUE_BRANCH instead of hard-coding for better > > readability. > > > > Signed-off-by: Jiang Liu <liuj97@gmail.com> > > --- > > include/linux/jump_label.h | 15 ++++++++++----- > > 1 file changed, 10 insertions(+), 5 deletions(-) > > Whilst this looks alright to me, I think we probably need an ack from Steve > in order to take changes to the core code. > > Will > Thanks for the heads up. I'll take a look at it. -- Steve ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v6 7/7] jump_label: use defined macros instead of hard-coding for better readability @ 2013-12-13 16:11 ` Steven Rostedt 0 siblings, 0 replies; 42+ messages in thread From: Steven Rostedt @ 2013-12-13 16:11 UTC (permalink / raw) To: Will Deacon Cc: Jiang Liu, Catalin Marinas, Sandeepa Prabhu, AKASHI Takahiro, Andrew Morton, Hannes Frederic Sowa, Andrew Jones, David S. Miller, Konrad Rzeszutek Wilk, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Jason Baron [ added Jason Baron too (the original author) ] On Fri, 13 Dec 2013 16:00:41 +0000 Will Deacon <will.deacon@arm.com> wrote: > On Tue, Dec 10, 2013 at 04:03:53PM +0000, Jiang Liu wrote: > > Use macro JUMP_LABEL_TRUE_BRANCH instead of hard-coding for better > > readability. > > > > Signed-off-by: Jiang Liu <liuj97@gmail.com> > > --- > > include/linux/jump_label.h | 15 ++++++++++----- > > 1 file changed, 10 insertions(+), 5 deletions(-) > > Whilst this looks alright to me, I think we probably need an ack from Steve > in order to take changes to the core code. > > Will > Thanks for the heads up. I'll take a look at it. -- Steve ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v6 7/7] jump_label: use defined macros instead of hard-coding for better readability 2013-12-10 16:03 ` Jiang Liu @ 2013-12-13 16:20 ` Steven Rostedt -1 siblings, 0 replies; 42+ messages in thread From: Steven Rostedt @ 2013-12-13 16:20 UTC (permalink / raw) To: linux-arm-kernel On Wed, 11 Dec 2013 00:03:53 +0800 Jiang Liu <liuj97@gmail.com> wrote: > Use macro JUMP_LABEL_TRUE_BRANCH instead of hard-coding for better > readability. Looks good, just a little nit. (see below) > > Signed-off-by: Jiang Liu <liuj97@gmail.com> > --- > include/linux/jump_label.h | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h > index 3999977..192ccba 100644 > --- a/include/linux/jump_label.h > +++ b/include/linux/jump_label.h > @@ -81,18 +81,21 @@ struct module; > #include <linux/atomic.h> > #ifdef HAVE_JUMP_LABEL > > -#define JUMP_LABEL_TRUE_BRANCH 1UL > +#define JUMP_LABEL_TYPE_FALSE_BRANCH 0UL > +#define JUMP_LABEL_TYPE_TRUE_BRANCH 1UL > +#define JUMP_LABEL_TYPE_MASK 1UL > > static > inline struct jump_entry *jump_label_get_entries(struct static_key *key) > { > return (struct jump_entry *)((unsigned long)key->entries > - & ~JUMP_LABEL_TRUE_BRANCH); > + & ~JUMP_LABEL_TYPE_MASK); > } > > static inline bool jump_label_get_branch_default(struct static_key *key) > { > - if ((unsigned long)key->entries & JUMP_LABEL_TRUE_BRANCH) > + if (((unsigned long)key->entries & JUMP_LABEL_TYPE_MASK) == > + JUMP_LABEL_TYPE_TRUE_BRANCH) > return true; > return false; > } > @@ -123,9 +126,11 @@ extern void static_key_slow_dec(struct static_key *key); > extern void jump_label_apply_nops(struct module *mod); > > #define STATIC_KEY_INIT_TRUE ((struct static_key) \ > - { .enabled = ATOMIC_INIT(1), .entries = (void *)1 }) > + { .enabled = ATOMIC_INIT(1), \ > + .entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH }) > #define STATIC_KEY_INIT_FALSE ((struct static_key) \ > - { .enabled = ATOMIC_INIT(0), .entries = (void *)0 }) > + { .enabled = ATOMIC_INIT(0), \ > + .entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH }) Can we prettify the backslashes like: #define STATIC_KEY_INIT_TRUE ((struct static_key) \ { .enabled = ATOMIC_INIT(1), \ .entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH }) #define STATIC_KEY_INIT_FALSE ((struct static_key) \ { .enabled = ATOMIC_INIT(0), \ .entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH }) Other than that. Acked-by: Steven Rostedt <rostedt@goodmis.org> -- Steve > > #else /* !HAVE_JUMP_LABEL */ > ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v6 7/7] jump_label: use defined macros instead of hard-coding for better readability @ 2013-12-13 16:20 ` Steven Rostedt 0 siblings, 0 replies; 42+ messages in thread From: Steven Rostedt @ 2013-12-13 16:20 UTC (permalink / raw) To: Jiang Liu Cc: Catalin Marinas, Will Deacon, Sandeepa Prabhu, AKASHI Takahiro, Andrew Morton, Hannes Frederic Sowa, Andrew Jones, David S. Miller, Konrad Rzeszutek Wilk, linux-kernel, linux-arm-kernel, Jason Baron On Wed, 11 Dec 2013 00:03:53 +0800 Jiang Liu <liuj97@gmail.com> wrote: > Use macro JUMP_LABEL_TRUE_BRANCH instead of hard-coding for better > readability. Looks good, just a little nit. (see below) > > Signed-off-by: Jiang Liu <liuj97@gmail.com> > --- > include/linux/jump_label.h | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h > index 3999977..192ccba 100644 > --- a/include/linux/jump_label.h > +++ b/include/linux/jump_label.h > @@ -81,18 +81,21 @@ struct module; > #include <linux/atomic.h> > #ifdef HAVE_JUMP_LABEL > > -#define JUMP_LABEL_TRUE_BRANCH 1UL > +#define JUMP_LABEL_TYPE_FALSE_BRANCH 0UL > +#define JUMP_LABEL_TYPE_TRUE_BRANCH 1UL > +#define JUMP_LABEL_TYPE_MASK 1UL > > static > inline struct jump_entry *jump_label_get_entries(struct static_key *key) > { > return (struct jump_entry *)((unsigned long)key->entries > - & ~JUMP_LABEL_TRUE_BRANCH); > + & ~JUMP_LABEL_TYPE_MASK); > } > > static inline bool jump_label_get_branch_default(struct static_key *key) > { > - if ((unsigned long)key->entries & JUMP_LABEL_TRUE_BRANCH) > + if (((unsigned long)key->entries & JUMP_LABEL_TYPE_MASK) == > + JUMP_LABEL_TYPE_TRUE_BRANCH) > return true; > return false; > } > @@ -123,9 +126,11 @@ extern void static_key_slow_dec(struct static_key *key); > extern void jump_label_apply_nops(struct module *mod); > > #define STATIC_KEY_INIT_TRUE ((struct static_key) \ > - { .enabled = ATOMIC_INIT(1), .entries = (void *)1 }) > + { .enabled = ATOMIC_INIT(1), \ > + .entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH }) > #define STATIC_KEY_INIT_FALSE ((struct static_key) \ > - { .enabled = ATOMIC_INIT(0), .entries = (void *)0 }) > + { .enabled = ATOMIC_INIT(0), \ > + .entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH }) Can we prettify the backslashes like: #define STATIC_KEY_INIT_TRUE ((struct static_key) \ { .enabled = ATOMIC_INIT(1), \ .entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH }) #define STATIC_KEY_INIT_FALSE ((struct static_key) \ { .enabled = ATOMIC_INIT(0), \ .entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH }) Other than that. Acked-by: Steven Rostedt <rostedt@goodmis.org> -- Steve > > #else /* !HAVE_JUMP_LABEL */ > ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v6 7/7] jump_label: use defined macros instead of hard-coding for better readability 2013-12-13 16:20 ` Steven Rostedt @ 2013-12-15 11:10 ` Jiang Liu -1 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-15 11:10 UTC (permalink / raw) To: linux-arm-kernel On 12/14/2013 12:20 AM, Steven Rostedt wrote: > On Wed, 11 Dec 2013 00:03:53 +0800 > Jiang Liu <liuj97@gmail.com> wrote: > >> Use macro JUMP_LABEL_TRUE_BRANCH instead of hard-coding for better >> readability. > > Looks good, just a little nit. (see below) > >> >> Signed-off-by: Jiang Liu <liuj97@gmail.com> >> --- >> include/linux/jump_label.h | 15 ++++++++++----- ... >> @@ -123,9 +126,11 @@ extern void static_key_slow_dec(struct static_key *key); >> extern void jump_label_apply_nops(struct module *mod); >> >> #define STATIC_KEY_INIT_TRUE ((struct static_key) \ >> - { .enabled = ATOMIC_INIT(1), .entries = (void *)1 }) >> + { .enabled = ATOMIC_INIT(1), \ >> + .entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH }) >> #define STATIC_KEY_INIT_FALSE ((struct static_key) \ >> - { .enabled = ATOMIC_INIT(0), .entries = (void *)0 }) >> + { .enabled = ATOMIC_INIT(0), \ >> + .entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH }) > > Can we prettify the backslashes like: > > #define STATIC_KEY_INIT_TRUE ((struct static_key) \ > { .enabled = ATOMIC_INIT(1), \ > .entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH }) > #define STATIC_KEY_INIT_FALSE ((struct static_key) \ > { .enabled = ATOMIC_INIT(0), \ > .entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH }) > > > Other than that. > > Acked-by: Steven Rostedt <rostedt@goodmis.org> Thanks Steve. Will make this change. > > -- Steve > >> >> #else /* !HAVE_JUMP_LABEL */ >> > ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v6 7/7] jump_label: use defined macros instead of hard-coding for better readability @ 2013-12-15 11:10 ` Jiang Liu 0 siblings, 0 replies; 42+ messages in thread From: Jiang Liu @ 2013-12-15 11:10 UTC (permalink / raw) To: Steven Rostedt Cc: Catalin Marinas, Will Deacon, Sandeepa Prabhu, AKASHI Takahiro, Andrew Morton, Hannes Frederic Sowa, Andrew Jones, David S. Miller, Konrad Rzeszutek Wilk, linux-kernel, linux-arm-kernel, Jason Baron On 12/14/2013 12:20 AM, Steven Rostedt wrote: > On Wed, 11 Dec 2013 00:03:53 +0800 > Jiang Liu <liuj97@gmail.com> wrote: > >> Use macro JUMP_LABEL_TRUE_BRANCH instead of hard-coding for better >> readability. > > Looks good, just a little nit. (see below) > >> >> Signed-off-by: Jiang Liu <liuj97@gmail.com> >> --- >> include/linux/jump_label.h | 15 ++++++++++----- ... >> @@ -123,9 +126,11 @@ extern void static_key_slow_dec(struct static_key *key); >> extern void jump_label_apply_nops(struct module *mod); >> >> #define STATIC_KEY_INIT_TRUE ((struct static_key) \ >> - { .enabled = ATOMIC_INIT(1), .entries = (void *)1 }) >> + { .enabled = ATOMIC_INIT(1), \ >> + .entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH }) >> #define STATIC_KEY_INIT_FALSE ((struct static_key) \ >> - { .enabled = ATOMIC_INIT(0), .entries = (void *)0 }) >> + { .enabled = ATOMIC_INIT(0), \ >> + .entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH }) > > Can we prettify the backslashes like: > > #define STATIC_KEY_INIT_TRUE ((struct static_key) \ > { .enabled = ATOMIC_INIT(1), \ > .entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH }) > #define STATIC_KEY_INIT_FALSE ((struct static_key) \ > { .enabled = ATOMIC_INIT(0), \ > .entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH }) > > > Other than that. > > Acked-by: Steven Rostedt <rostedt@goodmis.org> Thanks Steve. Will make this change. > > -- Steve > >> >> #else /* !HAVE_JUMP_LABEL */ >> > ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v6 7/7] jump_label: use defined macros instead of hard-coding for better readability 2013-12-15 11:10 ` Jiang Liu @ 2013-12-18 15:55 ` Jason Baron -1 siblings, 0 replies; 42+ messages in thread From: Jason Baron @ 2013-12-18 15:55 UTC (permalink / raw) To: linux-arm-kernel On 12/15/2013 06:10 AM, Jiang Liu wrote: > On 12/14/2013 12:20 AM, Steven Rostedt wrote: >> On Wed, 11 Dec 2013 00:03:53 +0800 >> Jiang Liu <liuj97@gmail.com> wrote: >> >>> Use macro JUMP_LABEL_TRUE_BRANCH instead of hard-coding for better >>> readability. >> Looks good, just a little nit. (see below) >> >>> Signed-off-by: Jiang Liu <liuj97@gmail.com> >>> --- >>> include/linux/jump_label.h | 15 ++++++++++----- > ... >>> @@ -123,9 +126,11 @@ extern void static_key_slow_dec(struct static_key *key); >>> extern void jump_label_apply_nops(struct module *mod); >>> >>> #define STATIC_KEY_INIT_TRUE ((struct static_key) \ >>> - { .enabled = ATOMIC_INIT(1), .entries = (void *)1 }) >>> + { .enabled = ATOMIC_INIT(1), \ >>> + .entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH }) >>> #define STATIC_KEY_INIT_FALSE ((struct static_key) \ >>> - { .enabled = ATOMIC_INIT(0), .entries = (void *)0 }) >>> + { .enabled = ATOMIC_INIT(0), \ >>> + .entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH }) >> Can we prettify the backslashes like: >> >> #define STATIC_KEY_INIT_TRUE ((struct static_key) \ >> { .enabled = ATOMIC_INIT(1), \ >> .entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH }) >> #define STATIC_KEY_INIT_FALSE ((struct static_key) \ >> { .enabled = ATOMIC_INIT(0), \ >> .entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH }) >> >> >> Other than that. >> >> Acked-by: Steven Rostedt <rostedt@goodmis.org> > Thanks Steve. > Will make this change. Works for me too, feel free to add: Acked-by: Jason Baron <jbaron@akamai.com> Thanks, -Jason ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v6 7/7] jump_label: use defined macros instead of hard-coding for better readability @ 2013-12-18 15:55 ` Jason Baron 0 siblings, 0 replies; 42+ messages in thread From: Jason Baron @ 2013-12-18 15:55 UTC (permalink / raw) To: Jiang Liu, Steven Rostedt Cc: Catalin Marinas, Will Deacon, Sandeepa Prabhu, AKASHI Takahiro, Andrew Morton, Hannes Frederic Sowa, Andrew Jones, David S. Miller, Konrad Rzeszutek Wilk, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org On 12/15/2013 06:10 AM, Jiang Liu wrote: > On 12/14/2013 12:20 AM, Steven Rostedt wrote: >> On Wed, 11 Dec 2013 00:03:53 +0800 >> Jiang Liu <liuj97@gmail.com> wrote: >> >>> Use macro JUMP_LABEL_TRUE_BRANCH instead of hard-coding for better >>> readability. >> Looks good, just a little nit. (see below) >> >>> Signed-off-by: Jiang Liu <liuj97@gmail.com> >>> --- >>> include/linux/jump_label.h | 15 ++++++++++----- > ... >>> @@ -123,9 +126,11 @@ extern void static_key_slow_dec(struct static_key *key); >>> extern void jump_label_apply_nops(struct module *mod); >>> >>> #define STATIC_KEY_INIT_TRUE ((struct static_key) \ >>> - { .enabled = ATOMIC_INIT(1), .entries = (void *)1 }) >>> + { .enabled = ATOMIC_INIT(1), \ >>> + .entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH }) >>> #define STATIC_KEY_INIT_FALSE ((struct static_key) \ >>> - { .enabled = ATOMIC_INIT(0), .entries = (void *)0 }) >>> + { .enabled = ATOMIC_INIT(0), \ >>> + .entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH }) >> Can we prettify the backslashes like: >> >> #define STATIC_KEY_INIT_TRUE ((struct static_key) \ >> { .enabled = ATOMIC_INIT(1), \ >> .entries = (void *)JUMP_LABEL_TYPE_TRUE_BRANCH }) >> #define STATIC_KEY_INIT_FALSE ((struct static_key) \ >> { .enabled = ATOMIC_INIT(0), \ >> .entries = (void *)JUMP_LABEL_TYPE_FALSE_BRANCH }) >> >> >> Other than that. >> >> Acked-by: Steven Rostedt <rostedt@goodmis.org> > Thanks Steve. > Will make this change. Works for me too, feel free to add: Acked-by: Jason Baron <jbaron@akamai.com> Thanks, -Jason ^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2013-12-18 15:55 UTC | newest]
Thread overview: 42+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-10 16:03 [PATCH v6 0/7] Optimize jump label implementation for ARM64 Jiang Liu
2013-12-10 16:03 ` Jiang Liu
2013-12-10 16:03 ` [PATCH v6 1/7] arm64: introduce basic aarch64 instruction decoding helpers Jiang Liu
2013-12-10 16:03 ` Jiang Liu
2013-12-10 16:03 ` [PATCH v6 2/7] arm64: introduce interfaces to hotpatch kernel and module code Jiang Liu
2013-12-10 16:03 ` Jiang Liu
2013-12-13 15:54 ` Will Deacon
2013-12-13 15:54 ` Will Deacon
2013-12-10 16:03 ` [PATCH v6 3/7] arm64: move encode_insn_immediate() from module.c to insn.c Jiang Liu
2013-12-10 16:03 ` Jiang Liu
2013-12-10 16:03 ` [PATCH v6 4/7] arm64: introduce aarch64_insn_gen_{nop|branch_imm}() helper functions Jiang Liu
2013-12-10 16:03 ` Jiang Liu
2013-12-13 15:58 ` Will Deacon
2013-12-13 15:58 ` Will Deacon
2013-12-10 16:03 ` [PATCH v6 5/7] arm64, jump label: detect %c support for ARM64 Jiang Liu
2013-12-10 16:03 ` Jiang Liu
2013-12-10 16:03 ` [PATCH v6 6/7] arm64, jump label: optimize jump label implementation Jiang Liu
2013-12-10 16:03 ` Jiang Liu
2013-12-13 15:41 ` Will Deacon
2013-12-13 15:41 ` Will Deacon
2013-12-13 15:50 ` Steven Rostedt
2013-12-13 15:50 ` Steven Rostedt
2013-12-13 15:52 ` Steven Rostedt
2013-12-13 15:52 ` Steven Rostedt
2013-12-15 11:07 ` Jiang Liu
2013-12-15 11:07 ` Jiang Liu
2013-12-13 15:55 ` Will Deacon
2013-12-13 15:55 ` Will Deacon
2013-12-15 11:07 ` Jiang Liu
2013-12-15 11:07 ` Jiang Liu
2013-12-10 16:03 ` [PATCH v6 7/7] jump_label: use defined macros instead of hard-coding for better readability Jiang Liu
2013-12-10 16:03 ` Jiang Liu
2013-12-13 16:00 ` Will Deacon
2013-12-13 16:00 ` Will Deacon
2013-12-13 16:11 ` Steven Rostedt
2013-12-13 16:11 ` Steven Rostedt
2013-12-13 16:20 ` Steven Rostedt
2013-12-13 16:20 ` Steven Rostedt
2013-12-15 11:10 ` Jiang Liu
2013-12-15 11:10 ` Jiang Liu
2013-12-18 15:55 ` Jason Baron
2013-12-18 15:55 ` Jason Baron
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.