* [PATCH v2] LoongArch: Add jump-label implementation
@ 2023-05-10 9:16 Youling Tang
2023-05-10 9:27 ` Peter Zijlstra
2023-05-10 9:28 ` WANG Xuerui
0 siblings, 2 replies; 10+ messages in thread
From: Youling Tang @ 2023-05-10 9:16 UTC (permalink / raw)
To: Huacai Chen
Cc: Jonathan Corbet, Peter Zijlstra, Josh Poimboeuf, Jason Baron,
WANG Xuerui, Zhangjin Wu, linux-doc, linux-kernel, loongarch
Add jump-label implementation based on the ARM64 version.
Signed-off-by: Youling Tang <tangyouling@loongson.cn>
---
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 |
| 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
+
+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)
+ 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);
+}
--
2.37.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] LoongArch: Add jump-label implementation
2023-05-10 9:16 [PATCH v2] LoongArch: Add jump-label implementation Youling Tang
@ 2023-05-10 9:27 ` Peter Zijlstra
2023-05-10 9:34 ` WANG Xuerui
2023-05-11 1:32 ` Youling Tang
2023-05-10 9:28 ` WANG Xuerui
1 sibling, 2 replies; 10+ messages in thread
From: Peter Zijlstra @ 2023-05-10 9:27 UTC (permalink / raw)
To: Youling Tang
Cc: Huacai Chen, Jonathan Corbet, Josh Poimboeuf, Jason Baron,
WANG Xuerui, Zhangjin Wu, linux-doc, linux-kernel, loongarch
On Wed, May 10, 2023 at 05:16:46PM +0800, Youling Tang wrote:
> Add jump-label implementation based on the ARM64 version.
>
> Signed-off-by: Youling Tang <tangyouling@loongson.cn>
> 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
> +
> +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;
> +}
Seems simple enough; one change I did a while ago for the x86 version is
to put the __jump_table entry generation in a macro so it could be
shared between the (3 for x86) variants.
Not saying you have to do that, just saying it's an option.
> +#endif /* __ASSEMBLY__ */
> +#endif /* __ASM_JUMP_LABEL_H */
> 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)
> + 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);
> +}
This all implies Loongarch is fine with the nop<->b transition (much
like arm64 is), but I found no actual mention of what transitions are
valid for the architecture in your inst.c file -- perhaps you could put
a small comment there to elucidate the occasional reader that doesn't
have your arch manual memorized?
Anyway, as with most RISC implementations it's short and sweet.
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] LoongArch: Add jump-label implementation
2023-05-10 9:16 [PATCH v2] LoongArch: Add jump-label implementation Youling Tang
2023-05-10 9:27 ` Peter Zijlstra
@ 2023-05-10 9:28 ` WANG Xuerui
2023-05-11 1:33 ` Youling Tang
1 sibling, 1 reply; 10+ messages in thread
From: WANG Xuerui @ 2023-05-10 9:28 UTC (permalink / raw)
To: Youling Tang, Huacai Chen
Cc: Jonathan Corbet, Peter Zijlstra, Josh Poimboeuf, Jason Baron,
Zhangjin Wu, linux-doc, linux-kernel, loongarch
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.
>
> Signed-off-by: Youling Tang <tangyouling@loongson.cn>
> ---
> 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.
> +
> +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.
> + 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);
> +}
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] LoongArch: Add jump-label implementation
2023-05-10 9:27 ` Peter Zijlstra
@ 2023-05-10 9:34 ` WANG Xuerui
2023-05-10 11:23 ` Peter Zijlstra
2023-05-11 1:32 ` Youling Tang
1 sibling, 1 reply; 10+ messages in thread
From: WANG Xuerui @ 2023-05-10 9:34 UTC (permalink / raw)
To: Peter Zijlstra, Youling Tang
Cc: Huacai Chen, Jonathan Corbet, Josh Poimboeuf, Jason Baron,
Zhangjin Wu, linux-doc, linux-kernel, loongarch
Hi Peter,
My 2 cents:
On 2023/5/10 17:27, Peter Zijlstra wrote:
> On Wed, May 10, 2023 at 05:16:46PM +0800, Youling Tang wrote:
>> Add jump-label implementation based on the ARM64 version.
>>
>> Signed-off-by: Youling Tang <tangyouling@loongson.cn>
>
>> <snip>
>>
>> + if (type == JUMP_LABEL_JMP)
>> + 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);
>> +}
>
> This all implies Loongarch is fine with the nop<->b transition (much
> like arm64 is), but I found no actual mention of what transitions are
> valid for the architecture in your inst.c file -- perhaps you could put
> a small comment there to elucidate the occasional reader that doesn't
> have your arch manual memorized?
Do you mean by "valid transition" something like "what kind of
self-modification is architecturally sound, taking ICache /
micro-architecture behavior etc. into consideration"? If so, I'd say
things would be fine in LoongArch as long as an instruction fetch
barrier is used. Maybe Youling can confirm and mention this in the next
revision.
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] LoongArch: Add jump-label implementation
2023-05-10 9:34 ` WANG Xuerui
@ 2023-05-10 11:23 ` Peter Zijlstra
0 siblings, 0 replies; 10+ messages in thread
From: Peter Zijlstra @ 2023-05-10 11:23 UTC (permalink / raw)
To: WANG Xuerui
Cc: Youling Tang, Huacai Chen, Jonathan Corbet, Josh Poimboeuf,
Jason Baron, Zhangjin Wu, linux-doc, linux-kernel, loongarch
On Wed, May 10, 2023 at 05:34:33PM +0800, WANG Xuerui wrote:
> Hi Peter,
>
> My 2 cents:
>
> On 2023/5/10 17:27, Peter Zijlstra wrote:
> > On Wed, May 10, 2023 at 05:16:46PM +0800, Youling Tang wrote:
> > > Add jump-label implementation based on the ARM64 version.
> > >
> > > Signed-off-by: Youling Tang <tangyouling@loongson.cn>
> >
> > > <snip>
> > >
> > > + if (type == JUMP_LABEL_JMP)
> > > + 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);
> > > +}
> >
> > This all implies Loongarch is fine with the nop<->b transition (much
> > like arm64 is), but I found no actual mention of what transitions are
> > valid for the architecture in your inst.c file -- perhaps you could put
> > a small comment there to elucidate the occasional reader that doesn't
> > have your arch manual memorized?
>
> Do you mean by "valid transition" something like "what kind of
> self-modification is architecturally sound, taking ICache /
> micro-architecture behavior etc. into consideration"?
Yes that. There are definitely architectures that have limitations on
what instructions can be hot-patched in the face of concurrent execution
without jumping through too many hoops.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] LoongArch: Add jump-label implementation
2023-05-10 9:27 ` Peter Zijlstra
2023-05-10 9:34 ` WANG Xuerui
@ 2023-05-11 1:32 ` Youling Tang
1 sibling, 0 replies; 10+ messages in thread
From: Youling Tang @ 2023-05-11 1:32 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Huacai Chen, Jonathan Corbet, Josh Poimboeuf, Jason Baron,
WANG Xuerui, Zhangjin Wu, linux-doc, linux-kernel, loongarch
Hi, Peter
On 05/10/2023 05:27 PM, Peter Zijlstra wrote:
> On Wed, May 10, 2023 at 05:16:46PM +0800, Youling Tang wrote:
>> Add jump-label implementation based on the ARM64 version.
>>
>> Signed-off-by: Youling Tang <tangyouling@loongson.cn>
>
>> 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
>> +
>> +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;
>> +}
>
> Seems simple enough; one change I did a while ago for the x86 version is
> to put the __jump_table entry generation in a macro so it could be
> shared between the (3 for x86) variants.
Looks better, I will define JUMP_TABLE_ENTRY macro so that
arch_static_branch and arch_static_branch_jump can share.
Thanks,
Youling.
>
> Not saying you have to do that, just saying it's an option.
>
>> +#endif /* __ASSEMBLY__ */
>> +#endif /* __ASM_JUMP_LABEL_H */
>
>> 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)
>> + 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);
>> +}
>
> This all implies Loongarch is fine with the nop<->b transition (much
> like arm64 is), but I found no actual mention of what transitions are
> valid for the architecture in your inst.c file -- perhaps you could put
> a small comment there to elucidate the occasional reader that doesn't
> have your arch manual memorized?
>
>
> Anyway, as with most RISC implementations it's short and sweet.
>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] LoongArch: Add jump-label implementation
2023-05-10 9:28 ` WANG Xuerui
@ 2023-05-11 1:33 ` Youling Tang
2023-05-11 7:43 ` Peter Zijlstra
2023-05-11 10:21 ` David Laight
0 siblings, 2 replies; 10+ messages in thread
From: Youling Tang @ 2023-05-11 1:33 UTC (permalink / raw)
To: WANG Xuerui
Cc: Huacai Chen, Jonathan Corbet, Peter Zijlstra, Josh Poimboeuf,
Jason Baron, Zhangjin Wu, linux-doc, linux-kernel, loongarch
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@loongson.cn>
>> ---
>> 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@intel.com/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);
>> +}
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] LoongArch: Add jump-label implementation
2023-05-11 1:33 ` Youling Tang
@ 2023-05-11 7:43 ` Peter Zijlstra
2023-05-11 10:07 ` WANG Xuerui
2023-05-11 10:21 ` David Laight
1 sibling, 1 reply; 10+ messages in thread
From: Peter Zijlstra @ 2023-05-11 7:43 UTC (permalink / raw)
To: Youling Tang
Cc: WANG Xuerui, Huacai Chen, Jonathan Corbet, Josh Poimboeuf,
Jason Baron, Zhangjin Wu, linux-doc, linux-kernel, loongarch
On Thu, May 11, 2023 at 09:33:37AM +0800, Youling Tang wrote:
> > > +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.
IIRC we used an enum with descriptive names instead of a boolean because
true/false just doesn't tell you much.
The whole thing fundamentally is a boolean descision though, either
you write a JMP or a NOP, jump-labels don't have more options.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] LoongArch: Add jump-label implementation
2023-05-11 7:43 ` Peter Zijlstra
@ 2023-05-11 10:07 ` WANG Xuerui
0 siblings, 0 replies; 10+ messages in thread
From: WANG Xuerui @ 2023-05-11 10:07 UTC (permalink / raw)
To: Peter Zijlstra, Youling Tang
Cc: Huacai Chen, Jonathan Corbet, Josh Poimboeuf, Jason Baron,
Zhangjin Wu, linux-doc, linux-kernel, loongarch
On 2023/5/11 15:43, Peter Zijlstra wrote:
> On Thu, May 11, 2023 at 09:33:37AM +0800, Youling Tang wrote:
>
>>>> +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.
>
> IIRC we used an enum with descriptive names instead of a boolean because
> true/false just doesn't tell you much.
>
> The whole thing fundamentally is a boolean descision though, either
> you write a JMP or a NOP, jump-labels don't have more options.
Ah thanks for the background. My previous suggestion is just kinda
generally applicable software engineering best practice, so if the
actual enum is unlikely to get >2 variants then it should be fine to
keep using "if". Youling, feel free to ignore the piece of comment, and
sorry for not doing my archaeology beforehand. :)
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v2] LoongArch: Add jump-label implementation
2023-05-11 1:33 ` Youling Tang
2023-05-11 7:43 ` Peter Zijlstra
@ 2023-05-11 10:21 ` David Laight
1 sibling, 0 replies; 10+ messages in thread
From: David Laight @ 2023-05-11 10:21 UTC (permalink / raw)
To: 'Youling Tang', WANG Xuerui
Cc: Huacai Chen, Jonathan Corbet, Peter Zijlstra, Josh Poimboeuf,
Jason Baron, Zhangjin Wu, linux-doc, linux-kernel, loongarch
From: Youling Tang
> Sent: 11 May 2023 02:34
...
> >> + 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.
The generated code will be pretty much the same.
Even if the compiler is allowed generate a jump table
(which is almost certainly disabled) it won't if there
are only two cases.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-05-11 10:21 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-10 9:16 [PATCH v2] LoongArch: Add jump-label implementation Youling Tang
2023-05-10 9:27 ` Peter Zijlstra
2023-05-10 9:34 ` WANG Xuerui
2023-05-10 11:23 ` Peter Zijlstra
2023-05-11 1:32 ` Youling Tang
2023-05-10 9:28 ` WANG Xuerui
2023-05-11 1:33 ` Youling Tang
2023-05-11 7:43 ` Peter Zijlstra
2023-05-11 10:07 ` WANG Xuerui
2023-05-11 10:21 ` David Laight
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).