* [RFC] riscv: Add jump-label implementation
@ 2020-07-02 20:07 Emil Renner Berthing
2020-07-03 15:42 ` Emil Renner Berthing
0 siblings, 1 reply; 5+ messages in thread
From: Emil Renner Berthing @ 2020-07-02 20:07 UTC (permalink / raw)
To: linux-riscv
Cc: Emil Renner Berthing, Palmer Dabbelt, Paul Walmsley, Anup Patel,
Atish Patra, linux-kernel
Add basic jump-label implementation heavily based
on the ARM64 version.
Tested on the HiFive Unleashed.
Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
---
This seems to work on my HiFive Unleashed. At least boots, runs fine
and the static key self-tests doesn't complain, but I'm sure I've missed
something, so I'm sending this as an RFC.
/Emil
.../core/jump-labels/arch-support.txt | 2 +-
arch/riscv/Kconfig | 2 +
arch/riscv/include/asm/jump_label.h | 59 +++++++++++++++++++
arch/riscv/kernel/Makefile | 2 +
arch/riscv/kernel/jump_label.c | 44 ++++++++++++++
5 files changed, 108 insertions(+), 1 deletion(-)
create mode 100644 arch/riscv/include/asm/jump_label.h
create mode 100644 arch/riscv/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 632a1c7aefa2..760243d18ed7 100644
--- a/Documentation/features/core/jump-labels/arch-support.txt
+++ b/Documentation/features/core/jump-labels/arch-support.txt
@@ -23,7 +23,7 @@
| openrisc: | TODO |
| parisc: | ok |
| powerpc: | ok |
- | riscv: | TODO |
+ | riscv: | ok |
| s390: | ok |
| sh: | TODO |
| sparc: | ok |
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index fd639937e251..d2f5c53fdc19 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -46,6 +46,8 @@ config RISCV
select GENERIC_TIME_VSYSCALL if MMU && 64BIT
select HANDLE_DOMAIN_IRQ
select HAVE_ARCH_AUDITSYSCALL
+ select HAVE_ARCH_JUMP_LABEL
+ select HAVE_ARCH_JUMP_LABEL_RELATIVE
select HAVE_ARCH_KASAN if MMU && 64BIT
select HAVE_ARCH_KGDB
select HAVE_ARCH_KGDB_QXFER_PKT
diff --git a/arch/riscv/include/asm/jump_label.h b/arch/riscv/include/asm/jump_label.h
new file mode 100644
index 000000000000..29be6d351866
--- /dev/null
+++ b/arch/riscv/include/asm/jump_label.h
@@ -0,0 +1,59 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 Emil Renner Berthing
+ *
+ * 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 *key,
+ bool branch)
+{
+ asm_volatile_goto(
+ " .option push \n\n"
+ " .option norelax \n\n"
+ " .option norvc \n\n"
+ "1: nop \n\t"
+ " .option pop \n\n"
+ " .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 ? 1 : 0]) : : l_yes);
+
+ return false;
+l_yes:
+ return true;
+}
+
+static __always_inline bool arch_static_branch_jump(struct static_key *key,
+ bool branch)
+{
+ asm_volatile_goto(
+ " .option push \n\n"
+ " .option norelax \n\n"
+ " .option norvc \n\n"
+ "1: jal zero, %l[l_yes] \n\t"
+ " .option pop \n\n"
+ " .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 ? 1 : 0]) : : l_yes);
+
+ return false;
+l_yes:
+ return true;
+}
+
+#endif /* __ASSEMBLY__ */
+#endif /* __ASM_JUMP_LABEL_H */
diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index b355cf485671..a5287ab9f7f2 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -53,4 +53,6 @@ endif
obj-$(CONFIG_HOTPLUG_CPU) += cpu-hotplug.o
obj-$(CONFIG_KGDB) += kgdb.o
+obj-$(CONFIG_JUMP_LABEL) += jump_label.o
+
clean:
diff --git a/arch/riscv/kernel/jump_label.c b/arch/riscv/kernel/jump_label.c
new file mode 100644
index 000000000000..55b2d742efe1
--- /dev/null
+++ b/arch/riscv/kernel/jump_label.c
@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2020 Emil Renner Berthing
+ *
+ * Based on arch/arm64/kernel/jump_label.c
+ */
+#include <linux/kernel.h>
+#include <linux/jump_label.h>
+#include <asm/patch.h>
+
+#define RISCV_INSN_NOP 0x00000013
+#define RISCV_INSN_JAL 0x0000006f
+
+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) {
+ u32 offset = jump_entry_target(entry) - jump_entry_code(entry);
+
+ insn = RISCV_INSN_JAL |
+ ((offset & GENMASK(19, 12)) << (12 - 12)) |
+ ((offset & GENMASK(11, 11)) << (20 - 11)) |
+ ((offset & GENMASK(10, 1)) << (21 - 1)) |
+ ((offset & GENMASK(20, 20)) << (31 - 20));
+ } else
+ insn = RISCV_INSN_NOP;
+
+ patch_text_nosync(addr, &insn, sizeof(insn));
+}
+
+void arch_jump_label_transform_static(struct jump_entry *entry,
+ enum jump_label_type type)
+{
+ /*
+ * We use the same instructions in the arch_static_branch and
+ * arch_static_branch_jump inline functions, so there's no
+ * need to patch them up here.
+ * The core will call arch_jump_label_transform when those
+ * instructions need to be replaced.
+ */
+}
--
2.27.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC] riscv: Add jump-label implementation
2020-07-02 20:07 [RFC] riscv: Add jump-label implementation Emil Renner Berthing
@ 2020-07-03 15:42 ` Emil Renner Berthing
2020-07-04 11:22 ` Björn Töpel
0 siblings, 1 reply; 5+ messages in thread
From: Emil Renner Berthing @ 2020-07-03 15:42 UTC (permalink / raw)
To: linux-riscv
Cc: Palmer Dabbelt, Paul Walmsley, Anup Patel, Atish Patra,
Linux Kernel Mailing List
On Thu, 2 Jul 2020 at 22:07, Emil Renner Berthing <kernel@esmil.dk> wrote:
>
> Add basic jump-label implementation heavily based
> on the ARM64 version.
>
> Tested on the HiFive Unleashed.
>
> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> ---
>
> This seems to work on my HiFive Unleashed. At least boots, runs fine
> and the static key self-tests doesn't complain, but I'm sure I've missed
> something, so I'm sending this as an RFC.
>
> /Emil
>
> .../core/jump-labels/arch-support.txt | 2 +-
> arch/riscv/Kconfig | 2 +
> arch/riscv/include/asm/jump_label.h | 59 +++++++++++++++++++
> arch/riscv/kernel/Makefile | 2 +
> arch/riscv/kernel/jump_label.c | 44 ++++++++++++++
> 5 files changed, 108 insertions(+), 1 deletion(-)
> create mode 100644 arch/riscv/include/asm/jump_label.h
> create mode 100644 arch/riscv/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 632a1c7aefa2..760243d18ed7 100644
> --- a/Documentation/features/core/jump-labels/arch-support.txt
> +++ b/Documentation/features/core/jump-labels/arch-support.txt
> @@ -23,7 +23,7 @@
> | openrisc: | TODO |
> | parisc: | ok |
> | powerpc: | ok |
> - | riscv: | TODO |
> + | riscv: | ok |
> | s390: | ok |
> | sh: | TODO |
> | sparc: | ok |
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index fd639937e251..d2f5c53fdc19 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -46,6 +46,8 @@ config RISCV
> select GENERIC_TIME_VSYSCALL if MMU && 64BIT
> select HANDLE_DOMAIN_IRQ
> select HAVE_ARCH_AUDITSYSCALL
> + select HAVE_ARCH_JUMP_LABEL
> + select HAVE_ARCH_JUMP_LABEL_RELATIVE
> select HAVE_ARCH_KASAN if MMU && 64BIT
> select HAVE_ARCH_KGDB
> select HAVE_ARCH_KGDB_QXFER_PKT
> diff --git a/arch/riscv/include/asm/jump_label.h b/arch/riscv/include/asm/jump_label.h
> new file mode 100644
> index 000000000000..29be6d351866
> --- /dev/null
> +++ b/arch/riscv/include/asm/jump_label.h
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2020 Emil Renner Berthing
> + *
> + * 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 *key,
> + bool branch)
> +{
> + asm_volatile_goto(
> + " .option push \n\n"
> + " .option norelax \n\n"
> + " .option norvc \n\n"
> + "1: nop \n\t"
> + " .option pop \n\n"
> + " .pushsection __jump_table, \"aw\" \n\t"
> + " .align 3 \n\t"
> + " .long 1b - ., %l[l_yes] - . \n\t"
> + " .quad %0 - . \n\t"
With HAVE_ARCH_JUMP_LABEL_RELATIVE we get
struct jump_entry {
s32 code;
s32 target;
long key;
}
..so this .quad and the one below should be replaced by the RISCV_PTR
macro to match the struct in 32bit kernels.
> + " .popsection \n\t"
> + : : "i"(&((char *)key)[branch ? 1 : 0]) : : l_yes);
> +
> + return false;
> +l_yes:
> + return true;
> +}
> +
> +static __always_inline bool arch_static_branch_jump(struct static_key *key,
> + bool branch)
> +{
> + asm_volatile_goto(
> + " .option push \n\n"
> + " .option norelax \n\n"
> + " .option norvc \n\n"
> + "1: jal zero, %l[l_yes] \n\t"
> + " .option pop \n\n"
> + " .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 ? 1 : 0]) : : l_yes);
> +
> + return false;
> +l_yes:
> + return true;
> +}
> +
> +#endif /* __ASSEMBLY__ */
> +#endif /* __ASM_JUMP_LABEL_H */
> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index b355cf485671..a5287ab9f7f2 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile
> @@ -53,4 +53,6 @@ endif
> obj-$(CONFIG_HOTPLUG_CPU) += cpu-hotplug.o
> obj-$(CONFIG_KGDB) += kgdb.o
>
> +obj-$(CONFIG_JUMP_LABEL) += jump_label.o
> +
> clean:
> diff --git a/arch/riscv/kernel/jump_label.c b/arch/riscv/kernel/jump_label.c
> new file mode 100644
> index 000000000000..55b2d742efe1
> --- /dev/null
> +++ b/arch/riscv/kernel/jump_label.c
> @@ -0,0 +1,44 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2020 Emil Renner Berthing
> + *
> + * Based on arch/arm64/kernel/jump_label.c
> + */
> +#include <linux/kernel.h>
> +#include <linux/jump_label.h>
> +#include <asm/patch.h>
> +
> +#define RISCV_INSN_NOP 0x00000013
> +#define RISCV_INSN_JAL 0x0000006f
> +
> +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) {
> + u32 offset = jump_entry_target(entry) - jump_entry_code(entry);
> +
> + insn = RISCV_INSN_JAL |
> + ((offset & GENMASK(19, 12)) << (12 - 12)) |
> + ((offset & GENMASK(11, 11)) << (20 - 11)) |
> + ((offset & GENMASK(10, 1)) << (21 - 1)) |
> + ((offset & GENMASK(20, 20)) << (31 - 20));
> + } else
> + insn = RISCV_INSN_NOP;
> +
> + patch_text_nosync(addr, &insn, sizeof(insn));
> +}
> +
> +void arch_jump_label_transform_static(struct jump_entry *entry,
> + enum jump_label_type type)
> +{
> + /*
> + * We use the same instructions in the arch_static_branch and
> + * arch_static_branch_jump inline functions, so there's no
> + * need to patch them up here.
> + * The core will call arch_jump_label_transform when those
> + * instructions need to be replaced.
> + */
> +}
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] riscv: Add jump-label implementation
2020-07-03 15:42 ` Emil Renner Berthing
@ 2020-07-04 11:22 ` Björn Töpel
2020-07-04 11:34 ` Emil Renner Berthing
0 siblings, 1 reply; 5+ messages in thread
From: Björn Töpel @ 2020-07-04 11:22 UTC (permalink / raw)
To: Emil Renner Berthing
Cc: linux-riscv, Palmer Dabbelt, Paul Walmsley, Anup Patel,
Atish Patra, Linux Kernel Mailing List
On Fri, 3 Jul 2020 at 17:43, Emil Renner Berthing <kernel@esmil.dk> wrote:
>
> On Thu, 2 Jul 2020 at 22:07, Emil Renner Berthing <kernel@esmil.dk> wrote:
> >
> > Add basic jump-label implementation heavily based
> > on the ARM64 version.
> >
> > Tested on the HiFive Unleashed.
> >
> > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> > ---
> >
> > This seems to work on my HiFive Unleashed. At least boots, runs fine
> > and the static key self-tests doesn't complain, but I'm sure I've missed
> > something, so I'm sending this as an RFC.
> >
> > /Emil
> >
> > .../core/jump-labels/arch-support.txt | 2 +-
> > arch/riscv/Kconfig | 2 +
> > arch/riscv/include/asm/jump_label.h | 59 +++++++++++++++++++
> > arch/riscv/kernel/Makefile | 2 +
> > arch/riscv/kernel/jump_label.c | 44 ++++++++++++++
> > 5 files changed, 108 insertions(+), 1 deletion(-)
> > create mode 100644 arch/riscv/include/asm/jump_label.h
> > create mode 100644 arch/riscv/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 632a1c7aefa2..760243d18ed7 100644
> > --- a/Documentation/features/core/jump-labels/arch-support.txt
> > +++ b/Documentation/features/core/jump-labels/arch-support.txt
> > @@ -23,7 +23,7 @@
> > | openrisc: | TODO |
> > | parisc: | ok |
> > | powerpc: | ok |
> > - | riscv: | TODO |
> > + | riscv: | ok |
> > | s390: | ok |
> > | sh: | TODO |
> > | sparc: | ok |
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index fd639937e251..d2f5c53fdc19 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -46,6 +46,8 @@ config RISCV
> > select GENERIC_TIME_VSYSCALL if MMU && 64BIT
> > select HANDLE_DOMAIN_IRQ
> > select HAVE_ARCH_AUDITSYSCALL
> > + select HAVE_ARCH_JUMP_LABEL
> > + select HAVE_ARCH_JUMP_LABEL_RELATIVE
> > select HAVE_ARCH_KASAN if MMU && 64BIT
> > select HAVE_ARCH_KGDB
> > select HAVE_ARCH_KGDB_QXFER_PKT
> > diff --git a/arch/riscv/include/asm/jump_label.h b/arch/riscv/include/asm/jump_label.h
> > new file mode 100644
> > index 000000000000..29be6d351866
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/jump_label.h
> > @@ -0,0 +1,59 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2020 Emil Renner Berthing
> > + *
> > + * 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 *key,
> > + bool branch)
> > +{
> > + asm_volatile_goto(
> > + " .option push \n\n"
> > + " .option norelax \n\n"
> > + " .option norvc \n\n"
> > + "1: nop \n\t"
> > + " .option pop \n\n"
> > + " .pushsection __jump_table, \"aw\" \n\t"
> > + " .align 3 \n\t"
> > + " .long 1b - ., %l[l_yes] - . \n\t"
> > + " .quad %0 - . \n\t"
>
> With HAVE_ARCH_JUMP_LABEL_RELATIVE we get
> struct jump_entry {
> s32 code;
> s32 target;
> long key;
> }
> ..so this .quad and the one below should be replaced by the RISCV_PTR
> macro to match the struct in 32bit kernels.
>
Indeed. And nice work! Can you respin the patch with the 32b fix
above, and also without the RFC tag?
Curious; Why is [branch ? 1 : 0] needed when coding the boolean into
the key pointer (arm64 is just [branch]). Different encoding of
booleans (branch in this case)?
Cheers,
Björn
> > + " .popsection \n\t"
> > + : : "i"(&((char *)key)[branch ? 1 : 0]) : : l_yes);
> > +
> > + return false;
> > +l_yes:
> > + return true;
> > +}
> > +
> > +static __always_inline bool arch_static_branch_jump(struct static_key *key,
> > + bool branch)
> > +{
> > + asm_volatile_goto(
> > + " .option push \n\n"
> > + " .option norelax \n\n"
> > + " .option norvc \n\n"
> > + "1: jal zero, %l[l_yes] \n\t"
> > + " .option pop \n\n"
> > + " .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 ? 1 : 0]) : : l_yes);
> > +
> > + return false;
> > +l_yes:
> > + return true;
> > +}
> > +
> > +#endif /* __ASSEMBLY__ */
> > +#endif /* __ASM_JUMP_LABEL_H */
> > diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> > index b355cf485671..a5287ab9f7f2 100644
> > --- a/arch/riscv/kernel/Makefile
> > +++ b/arch/riscv/kernel/Makefile
> > @@ -53,4 +53,6 @@ endif
> > obj-$(CONFIG_HOTPLUG_CPU) += cpu-hotplug.o
> > obj-$(CONFIG_KGDB) += kgdb.o
> >
> > +obj-$(CONFIG_JUMP_LABEL) += jump_label.o
> > +
> > clean:
> > diff --git a/arch/riscv/kernel/jump_label.c b/arch/riscv/kernel/jump_label.c
> > new file mode 100644
> > index 000000000000..55b2d742efe1
> > --- /dev/null
> > +++ b/arch/riscv/kernel/jump_label.c
> > @@ -0,0 +1,44 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2020 Emil Renner Berthing
> > + *
> > + * Based on arch/arm64/kernel/jump_label.c
> > + */
> > +#include <linux/kernel.h>
> > +#include <linux/jump_label.h>
> > +#include <asm/patch.h>
> > +
> > +#define RISCV_INSN_NOP 0x00000013
> > +#define RISCV_INSN_JAL 0x0000006f
> > +
> > +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) {
> > + u32 offset = jump_entry_target(entry) - jump_entry_code(entry);
> > +
> > + insn = RISCV_INSN_JAL |
> > + ((offset & GENMASK(19, 12)) << (12 - 12)) |
> > + ((offset & GENMASK(11, 11)) << (20 - 11)) |
> > + ((offset & GENMASK(10, 1)) << (21 - 1)) |
> > + ((offset & GENMASK(20, 20)) << (31 - 20));
> > + } else
> > + insn = RISCV_INSN_NOP;
> > +
> > + patch_text_nosync(addr, &insn, sizeof(insn));
> > +}
> > +
> > +void arch_jump_label_transform_static(struct jump_entry *entry,
> > + enum jump_label_type type)
> > +{
> > + /*
> > + * We use the same instructions in the arch_static_branch and
> > + * arch_static_branch_jump inline functions, so there's no
> > + * need to patch them up here.
> > + * The core will call arch_jump_label_transform when those
> > + * instructions need to be replaced.
> > + */
> > +}
> > --
> > 2.27.0
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] riscv: Add jump-label implementation
2020-07-04 11:22 ` Björn Töpel
@ 2020-07-04 11:34 ` Emil Renner Berthing
2020-07-04 12:09 ` Björn Töpel
0 siblings, 1 reply; 5+ messages in thread
From: Emil Renner Berthing @ 2020-07-04 11:34 UTC (permalink / raw)
To: Björn Töpel
Cc: linux-riscv, Palmer Dabbelt, Paul Walmsley, Anup Patel,
Atish Patra, Linux Kernel Mailing List
On Sat, 4 Jul 2020 at 13:23, Björn Töpel <bjorn.topel@gmail.com> wrote:
> On Fri, 3 Jul 2020 at 17:43, Emil Renner Berthing <kernel@esmil.dk> wrote:
> > On Thu, 2 Jul 2020 at 22:07, Emil Renner Berthing <kernel@esmil.dk> wrote:
> > >
> > > Add basic jump-label implementation heavily based
> > > on the ARM64 version.
> > >
> > > Tested on the HiFive Unleashed.
> > >
> > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> > > ---
> > >
> > > This seems to work on my HiFive Unleashed. At least boots, runs fine
> > > and the static key self-tests doesn't complain, but I'm sure I've missed
> > > something, so I'm sending this as an RFC.
> > >
> > > /Emil
> > >
> > > .../core/jump-labels/arch-support.txt | 2 +-
> > > arch/riscv/Kconfig | 2 +
> > > arch/riscv/include/asm/jump_label.h | 59 +++++++++++++++++++
> > > arch/riscv/kernel/Makefile | 2 +
> > > arch/riscv/kernel/jump_label.c | 44 ++++++++++++++
> > > 5 files changed, 108 insertions(+), 1 deletion(-)
> > > create mode 100644 arch/riscv/include/asm/jump_label.h
> > > create mode 100644 arch/riscv/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 632a1c7aefa2..760243d18ed7 100644
> > > --- a/Documentation/features/core/jump-labels/arch-support.txt
> > > +++ b/Documentation/features/core/jump-labels/arch-support.txt
> > > @@ -23,7 +23,7 @@
> > > | openrisc: | TODO |
> > > | parisc: | ok |
> > > | powerpc: | ok |
> > > - | riscv: | TODO |
> > > + | riscv: | ok |
> > > | s390: | ok |
> > > | sh: | TODO |
> > > | sparc: | ok |
> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > index fd639937e251..d2f5c53fdc19 100644
> > > --- a/arch/riscv/Kconfig
> > > +++ b/arch/riscv/Kconfig
> > > @@ -46,6 +46,8 @@ config RISCV
> > > select GENERIC_TIME_VSYSCALL if MMU && 64BIT
> > > select HANDLE_DOMAIN_IRQ
> > > select HAVE_ARCH_AUDITSYSCALL
> > > + select HAVE_ARCH_JUMP_LABEL
> > > + select HAVE_ARCH_JUMP_LABEL_RELATIVE
> > > select HAVE_ARCH_KASAN if MMU && 64BIT
> > > select HAVE_ARCH_KGDB
> > > select HAVE_ARCH_KGDB_QXFER_PKT
> > > diff --git a/arch/riscv/include/asm/jump_label.h b/arch/riscv/include/asm/jump_label.h
> > > new file mode 100644
> > > index 000000000000..29be6d351866
> > > --- /dev/null
> > > +++ b/arch/riscv/include/asm/jump_label.h
> > > @@ -0,0 +1,59 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +/*
> > > + * Copyright (C) 2020 Emil Renner Berthing
> > > + *
> > > + * 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 *key,
> > > + bool branch)
> > > +{
> > > + asm_volatile_goto(
> > > + " .option push \n\n"
> > > + " .option norelax \n\n"
> > > + " .option norvc \n\n"
> > > + "1: nop \n\t"
> > > + " .option pop \n\n"
> > > + " .pushsection __jump_table, \"aw\" \n\t"
> > > + " .align 3 \n\t"
> > > + " .long 1b - ., %l[l_yes] - . \n\t"
> > > + " .quad %0 - . \n\t"
> >
> > With HAVE_ARCH_JUMP_LABEL_RELATIVE we get
> > struct jump_entry {
> > s32 code;
> > s32 target;
> > long key;
> > }
> > ..so this .quad and the one below should be replaced by the RISCV_PTR
> > macro to match the struct in 32bit kernels.
> >
>
> Indeed. And nice work! Can you respin the patch with the 32b fix
> above, and also without the RFC tag?
Yes, of course. If you don't mind I'll wait a bit and let this collect
a bit more comments.
> Curious; Why is [branch ? 1 : 0] needed when coding the boolean into
> the key pointer (arm64 is just [branch]). Different encoding of
> booleans (branch in this case)?
No, that was just me being unsure exactly how bool works when used as
an index. After reading up on it it seems the original code is right,
you can actually trust that _Bool is either 0 or 1. I'll fix it in the
next version. Thanks!
/Emil
>
>
> Cheers,
> Björn
>
> > > + " .popsection \n\t"
> > > + : : "i"(&((char *)key)[branch ? 1 : 0]) : : l_yes);
> > > +
> > > + return false;
> > > +l_yes:
> > > + return true;
> > > +}
> > > +
> > > +static __always_inline bool arch_static_branch_jump(struct static_key *key,
> > > + bool branch)
> > > +{
> > > + asm_volatile_goto(
> > > + " .option push \n\n"
> > > + " .option norelax \n\n"
> > > + " .option norvc \n\n"
> > > + "1: jal zero, %l[l_yes] \n\t"
> > > + " .option pop \n\n"
> > > + " .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 ? 1 : 0]) : : l_yes);
> > > +
> > > + return false;
> > > +l_yes:
> > > + return true;
> > > +}
> > > +
> > > +#endif /* __ASSEMBLY__ */
> > > +#endif /* __ASM_JUMP_LABEL_H */
> > > diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> > > index b355cf485671..a5287ab9f7f2 100644
> > > --- a/arch/riscv/kernel/Makefile
> > > +++ b/arch/riscv/kernel/Makefile
> > > @@ -53,4 +53,6 @@ endif
> > > obj-$(CONFIG_HOTPLUG_CPU) += cpu-hotplug.o
> > > obj-$(CONFIG_KGDB) += kgdb.o
> > >
> > > +obj-$(CONFIG_JUMP_LABEL) += jump_label.o
> > > +
> > > clean:
> > > diff --git a/arch/riscv/kernel/jump_label.c b/arch/riscv/kernel/jump_label.c
> > > new file mode 100644
> > > index 000000000000..55b2d742efe1
> > > --- /dev/null
> > > +++ b/arch/riscv/kernel/jump_label.c
> > > @@ -0,0 +1,44 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Copyright (C) 2020 Emil Renner Berthing
> > > + *
> > > + * Based on arch/arm64/kernel/jump_label.c
> > > + */
> > > +#include <linux/kernel.h>
> > > +#include <linux/jump_label.h>
> > > +#include <asm/patch.h>
> > > +
> > > +#define RISCV_INSN_NOP 0x00000013
> > > +#define RISCV_INSN_JAL 0x0000006f
> > > +
> > > +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) {
> > > + u32 offset = jump_entry_target(entry) - jump_entry_code(entry);
> > > +
> > > + insn = RISCV_INSN_JAL |
> > > + ((offset & GENMASK(19, 12)) << (12 - 12)) |
> > > + ((offset & GENMASK(11, 11)) << (20 - 11)) |
> > > + ((offset & GENMASK(10, 1)) << (21 - 1)) |
> > > + ((offset & GENMASK(20, 20)) << (31 - 20));
> > > + } else
> > > + insn = RISCV_INSN_NOP;
> > > +
> > > + patch_text_nosync(addr, &insn, sizeof(insn));
> > > +}
> > > +
> > > +void arch_jump_label_transform_static(struct jump_entry *entry,
> > > + enum jump_label_type type)
> > > +{
> > > + /*
> > > + * We use the same instructions in the arch_static_branch and
> > > + * arch_static_branch_jump inline functions, so there's no
> > > + * need to patch them up here.
> > > + * The core will call arch_jump_label_transform when those
> > > + * instructions need to be replaced.
> > > + */
> > > +}
> > > --
> > > 2.27.0
> > >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] riscv: Add jump-label implementation
2020-07-04 11:34 ` Emil Renner Berthing
@ 2020-07-04 12:09 ` Björn Töpel
0 siblings, 0 replies; 5+ messages in thread
From: Björn Töpel @ 2020-07-04 12:09 UTC (permalink / raw)
To: Emil Renner Berthing
Cc: linux-riscv, Palmer Dabbelt, Paul Walmsley, Anup Patel,
Atish Patra, Linux Kernel Mailing List
On Sat, 4 Jul 2020 at 13:35, Emil Renner Berthing <kernel@esmil.dk> wrote:
>
> On Sat, 4 Jul 2020 at 13:23, Björn Töpel <bjorn.topel@gmail.com> wrote:
[...]
> > Indeed. And nice work! Can you respin the patch with the 32b fix
> > above, and also without the RFC tag?
>
> Yes, of course. If you don't mind I'll wait a bit and let this collect
> a bit more comments.
>
Certainly!
> > Curious; Why is [branch ? 1 : 0] needed when coding the boolean into
> > the key pointer (arm64 is just [branch]). Different encoding of
> > booleans (branch in this case)?
>
> No, that was just me being unsure exactly how bool works when used as
> an index. After reading up on it it seems the original code is right,
> you can actually trust that _Bool is either 0 or 1. I'll fix it in the
> next version. Thanks!
>
Cool! Thanks for clearing that up for me!
Cheers,
Björn
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-07-04 12:09 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-02 20:07 [RFC] riscv: Add jump-label implementation Emil Renner Berthing
2020-07-03 15:42 ` Emil Renner Berthing
2020-07-04 11:22 ` Björn Töpel
2020-07-04 11:34 ` Emil Renner Berthing
2020-07-04 12:09 ` Björn Töpel
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).