Re: [PATCH v2] LoongArch: Add jump-label implementation

From: Youling Tang
Date: Wed May 10 2023 - 21:33:52 EST

Hi, Xuerui

On 05/10/2023 05:28 PM, WANG Xuerui wrote:
Hi Youling,

On 2023/5/10 17:16, Youling Tang wrote:
Add jump-label implementation based on the ARM64 version.

"Add support for jump labels based on ..." sounds better IMO.

OK.



Signed-off-by: Youling Tang <tangyouling@xxxxxxxxxxx>
---
Changes in v2:
- Fix build errors.
- fix comment.

.../core/jump-labels/arch-support.txt | 2 +-
arch/loongarch/Kconfig | 2 +
arch/loongarch/configs/loongson3_defconfig | 1 +
arch/loongarch/include/asm/jump_label.h | 51 +++++++++++++++++++
arch/loongarch/kernel/Makefile | 2 +
arch/loongarch/kernel/jump_label.c | 23 +++++++++
6 files changed, 80 insertions(+), 1 deletion(-)
create mode 100644 arch/loongarch/include/asm/jump_label.h
create mode 100644 arch/loongarch/kernel/jump_label.c

diff --git a/Documentation/features/core/jump-labels/arch-support.txt
b/Documentation/features/core/jump-labels/arch-support.txt
index 2328eada3a49..94d9dece580f 100644
--- a/Documentation/features/core/jump-labels/arch-support.txt
+++ b/Documentation/features/core/jump-labels/arch-support.txt
@@ -13,7 +13,7 @@
| csky: | ok |
| hexagon: | TODO |
| ia64: | TODO |
- | loongarch: | TODO |
+ | loongarch: | ok |

+1 for updating the docs along with the implementation!

| m68k: | TODO |
| microblaze: | TODO |
| mips: | ok |
diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index d38b066fc931..193a959a5611 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -83,6 +83,8 @@ config LOONGARCH
select GPIOLIB
select HAS_IOPORT
select HAVE_ARCH_AUDITSYSCALL
+ select HAVE_ARCH_JUMP_LABEL
+ select HAVE_ARCH_JUMP_LABEL_RELATIVE
select HAVE_ARCH_MMAP_RND_BITS if MMU
select HAVE_ARCH_SECCOMP_FILTER
select HAVE_ARCH_TRACEHOOK
diff --git a/arch/loongarch/configs/loongson3_defconfig
b/arch/loongarch/configs/loongson3_defconfig
index 6cd26dd3c134..33a0f5f742f6 100644
--- a/arch/loongarch/configs/loongson3_defconfig
+++ b/arch/loongarch/configs/loongson3_defconfig
@@ -63,6 +63,7 @@ CONFIG_EFI_ZBOOT=y
CONFIG_EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER=y
CONFIG_EFI_CAPSULE_LOADER=m
CONFIG_EFI_TEST=m
+CONFIG_JUMP_LABEL=y
CONFIG_MODULES=y
CONFIG_MODULE_FORCE_LOAD=y
CONFIG_MODULE_UNLOAD=y
diff --git a/arch/loongarch/include/asm/jump_label.h
b/arch/loongarch/include/asm/jump_label.h
new file mode 100644
index 000000000000..2f9fdec256c5
--- /dev/null
+++ b/arch/loongarch/include/asm/jump_label.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2023 Loongson Technology Corporation Limited
+ *
+ * Based on arch/arm64/include/asm/jump_label.h
+ */
+#ifndef __ASM_JUMP_LABEL_H
+#define __ASM_JUMP_LABEL_H
+
+#ifndef __ASSEMBLY__
+
+#include <linux/types.h>
+
+#define JUMP_LABEL_NOP_SIZE 4

Saying LOONGARCH_INSN_SIZE here might be better for reducing redundancy,
although that'll necessitate an extra include of <asm/inst.h>. Leaving
the 4 alone won't cause much harm so I'm fine with either.

Using LOONGARCH_INSN_SIZE in v1, but causing build errors due to
inclusion of <asm/inst.h> [1].

So I removed the <asm/inst.h> include and used hardcoded 4.

[1]: https://lore.kernel.org/loongarch/202305100412.gazWW71q-lkp@xxxxxxxxx/T/#m0d8393aaf529aea0a4dcc5985469e698d63d66b3

+
+static __always_inline bool arch_static_branch(struct static_key *
const key,
+ const bool branch)
+{
+ asm_volatile_goto(
+ "1: nop \n\t"
+ " .pushsection __jump_table, \"aw\" \n\t"
+ " .align 3 \n\t"
+ " .long 1b - ., %l[l_yes] - . \n\t"
+ " .quad %0 - . \n\t"
+ " .popsection \n\t"
+ : : "i"(&((char *)key)[branch]) : : l_yes);
+
+ return false;
+l_yes:
+ return true;
+}
+
+static __always_inline bool arch_static_branch_jump(struct static_key
* const key,
+ const bool branch)
+{
+ asm_volatile_goto(
+ "1: b %l[l_yes] \n\t"
+ " .pushsection __jump_table, \"aw\" \n\t"
+ " .align 3 \n\t"
+ " .long 1b - ., %l[l_yes] - . \n\t"
+ " .quad %0 - . \n\t"
+ " .popsection \n\t"
+ : : "i"(&((char *)key)[branch]) : : l_yes);
+
+ return false;
+l_yes:
+ return true;
+}
+
+#endif /* __ASSEMBLY__ */
+#endif /* __ASM_JUMP_LABEL_H */
diff --git a/arch/loongarch/kernel/Makefile
b/arch/loongarch/kernel/Makefile
index 9a72d91cd104..64ea76f60e2c 100644
--- a/arch/loongarch/kernel/Makefile
+++ b/arch/loongarch/kernel/Makefile
@@ -54,4 +54,6 @@ obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
obj-$(CONFIG_KPROBES) += kprobes.o kprobes_trampoline.o
+obj-$(CONFIG_JUMP_LABEL) += jump_label.o
+
CPPFLAGS_vmlinux.lds := $(KBUILD_CFLAGS)
diff --git a/arch/loongarch/kernel/jump_label.c
b/arch/loongarch/kernel/jump_label.c
new file mode 100644
index 000000000000..b06245955f7a
--- /dev/null
+++ b/arch/loongarch/kernel/jump_label.c
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2023 Loongson Technology Corporation Limited
+ *
+ * Based on arch/arm64/kernel/jump_label.c
+ */
+#include <linux/jump_label.h>
+#include <linux/kernel.h>
+#include <asm/inst.h>
+
+void arch_jump_label_transform(struct jump_entry *entry,
+ enum jump_label_type type)
+{
+ void *addr = (void *)jump_entry_code(entry);
+ u32 insn;
+
+ if (type == JUMP_LABEL_JMP)

Please use a switch for dealing with enum-typed values.

Because the current type only has JUMP_LABEL_NOP and JUMP_LABEL_JMP,
using if may be simpler than switch.

Thanks,
Youling.

+ insn = larch_insn_gen_b(jump_entry_code(entry),
jump_entry_target(entry));
+ else
+ insn = larch_insn_gen_nop();
+
+ larch_insn_patch_text(addr, insn);
+}