* [PATCH -next V2 1/2] riscv: jump_label: Fixup unaligned arch_static_branch function
2022-12-10 10:09 [PATCH -next V2 0/2] riscv: jump_label: Fixup & Optimization guoren
@ 2022-12-10 10:09 ` guoren
2022-12-10 10:09 ` [PATCH -next V2 2/2] riscv: jump_label: Using c.j for CONFIG_RISCV_ISA_C guoren
1 sibling, 0 replies; 3+ messages in thread
From: guoren @ 2022-12-10 10:09 UTC (permalink / raw)
To: palmer, conor.Dooley, guoren, andy.chiu, greentime.hu,
paul.walmsley, peterz, rostedt, jrtc27
Cc: linux-kernel, linux-riscv
From: Andy Chiu <andy.chiu@sifive.com>
Runtime code patching must be done at a naturally aligned address, or we
may execute on a partial instruction.
We have encountered problems traced back to static jump functions during
the test. We switched the tracer randomly for every 1~5 seconds on a
dual-core QEMU setup and found the kernel sucking at a static branch
where it jumps to itself.
The reason is that the static branch was 2-byte but not 4-byte aligned.
Then, the kernel would patch the instruction, either J or NOP, with two
half-word stores if the machine does not have efficient unaligned
accesses. Thus, moments exist where half of the NOP mixes with the other
half of the J when transitioning the branch. In our particular case, on
a little-endian machine, the upper half of the NOP was mixed with the
lower part of the J when enabling the branch, resulting in a jump that
jumped to itself. Conversely, it would result in a HINT instruction when
disabling the branch, but it might not be observable.
ARM64 does not have this problem since all instructions must be 4-byte
aligned.
Fixes: ebc00dde8a97 ("riscv: Add jump-label implementation")
Link: https://lore.kernel.org/linux-riscv/20220913094252.3555240-6-andy.chiu@sifive.com/
Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
Reviewed-by: Greentime Hu <greentime.hu@sifive.com>
Signed-off-by: Guo Ren <guoren@kernel.org>
---
arch/riscv/include/asm/jump_label.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/riscv/include/asm/jump_label.h b/arch/riscv/include/asm/jump_label.h
index 38af2ec7b9bf..729991e8f782 100644
--- a/arch/riscv/include/asm/jump_label.h
+++ b/arch/riscv/include/asm/jump_label.h
@@ -18,6 +18,7 @@ static __always_inline bool arch_static_branch(struct static_key *key,
bool branch)
{
asm_volatile_goto(
+ " .align 2 \n\t"
" .option push \n\t"
" .option norelax \n\t"
" .option norvc \n\t"
@@ -39,6 +40,7 @@ static __always_inline bool arch_static_branch_jump(struct static_key *key,
bool branch)
{
asm_volatile_goto(
+ " .align 2 \n\t"
" .option push \n\t"
" .option norelax \n\t"
" .option norvc \n\t"
--
2.36.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH -next V2 2/2] riscv: jump_label: Using c.j for CONFIG_RISCV_ISA_C
2022-12-10 10:09 [PATCH -next V2 0/2] riscv: jump_label: Fixup & Optimization guoren
2022-12-10 10:09 ` [PATCH -next V2 1/2] riscv: jump_label: Fixup unaligned arch_static_branch function guoren
@ 2022-12-10 10:09 ` guoren
1 sibling, 0 replies; 3+ messages in thread
From: guoren @ 2022-12-10 10:09 UTC (permalink / raw)
To: palmer, conor.Dooley, guoren, andy.chiu, greentime.hu,
paul.walmsley, peterz, rostedt, jrtc27
Cc: linux-kernel, linux-riscv, Guo Ren
From: Guo Ren <guoren@linux.alibaba.com>
Reduce the size of the static branch instruction and prevent atomic
update problems. It would also reduce the range from 1MB to 4KB.
The range of 4K is enough for static branch.
Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Signed-off-by: Guo Ren <guoren@kernel.org>
Cc: Jessica Clarke <jrtc27@jrtc27.com>
---
arch/riscv/include/asm/jump_label.h | 16 +++++++++++----
arch/riscv/kernel/jump_label.c | 30 +++++++++++++++++++++++++++--
2 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/arch/riscv/include/asm/jump_label.h b/arch/riscv/include/asm/jump_label.h
index 729991e8f782..e5f772ad5887 100644
--- a/arch/riscv/include/asm/jump_label.h
+++ b/arch/riscv/include/asm/jump_label.h
@@ -12,17 +12,23 @@
#include <linux/types.h>
#include <asm/asm.h>
+#ifdef CONFIG_RISCV_ISA_C
+#define JUMP_LABEL_NOP_SIZE 2
+#else
#define JUMP_LABEL_NOP_SIZE 4
+#endif
static __always_inline bool arch_static_branch(struct static_key *key,
bool branch)
{
asm_volatile_goto(
- " .align 2 \n\t"
" .option push \n\t"
" .option norelax \n\t"
- " .option norvc \n\t"
+#ifdef CONFIG_RISCV_ISA_C
+ "1: c.nop \n\t"
+#else
"1: nop \n\t"
+#endif
" .option pop \n\t"
" .pushsection __jump_table, \"aw\" \n\t"
" .align " RISCV_LGPTR " \n\t"
@@ -40,11 +46,13 @@ static __always_inline bool arch_static_branch_jump(struct static_key *key,
bool branch)
{
asm_volatile_goto(
- " .align 2 \n\t"
" .option push \n\t"
" .option norelax \n\t"
- " .option norvc \n\t"
+#ifdef CONFIG_RISCV_ISA_C
+ "1: c.j %l[label] \n\t"
+#else
"1: jal zero, %l[label] \n\t"
+#endif
" .option pop \n\t"
" .pushsection __jump_table, \"aw\" \n\t"
" .align " RISCV_LGPTR " \n\t"
diff --git a/arch/riscv/kernel/jump_label.c b/arch/riscv/kernel/jump_label.c
index e6694759dbd0..08f42c49e3a0 100644
--- a/arch/riscv/kernel/jump_label.c
+++ b/arch/riscv/kernel/jump_label.c
@@ -11,26 +11,52 @@
#include <asm/bug.h>
#include <asm/patch.h>
+#ifdef CONFIG_RISCV_ISA_C
+#define RISCV_INSN_NOP 0x0001U
+#define RISCV_INSN_C_J 0xa001U
+#else
#define RISCV_INSN_NOP 0x00000013U
#define RISCV_INSN_JAL 0x0000006fU
+#endif
void arch_jump_label_transform(struct jump_entry *entry,
enum jump_label_type type)
{
void *addr = (void *)jump_entry_code(entry);
+#ifdef CONFIG_RISCV_ISA_C
+ u16 insn;
+#else
u32 insn;
+#endif
if (type == JUMP_LABEL_JMP) {
long offset = jump_entry_target(entry) - jump_entry_code(entry);
-
- if (WARN_ON(offset & 1 || offset < -524288 || offset >= 524288))
+ if (WARN_ON(offset & 1 || offset < -2048 || offset >= 2048))
return;
+#ifdef CONFIG_RISCV_ISA_C
+ /*
+ * 001 | imm[11|4|9:8|10|6|7|3:1|5] 01 - C.J
+ */
+ insn = RISCV_INSN_C_J |
+ (((u16)offset & GENMASK(5, 5)) >> (5 - 2)) |
+ (((u16)offset & GENMASK(3, 1)) << (3 - 1)) |
+ (((u16)offset & GENMASK(7, 7)) >> (7 - 6)) |
+ (((u16)offset & GENMASK(6, 6)) << (7 - 6)) |
+ (((u16)offset & GENMASK(10, 10)) >> (10 - 8)) |
+ (((u16)offset & GENMASK(9, 8)) << (9 - 8)) |
+ (((u16)offset & GENMASK(4, 4)) << (11 - 4)) |
+ (((u16)offset & GENMASK(11, 11)) << (12 - 11));
+#else
+ /*
+ * imm[20|10:1|11|19:12] | rd | 1101111 - JAL
+ */
insn = RISCV_INSN_JAL |
(((u32)offset & GENMASK(19, 12)) << (12 - 12)) |
(((u32)offset & GENMASK(11, 11)) << (20 - 11)) |
(((u32)offset & GENMASK(10, 1)) << (21 - 1)) |
(((u32)offset & GENMASK(20, 20)) << (31 - 20));
+#endif
} else {
insn = RISCV_INSN_NOP;
}
--
2.36.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 3+ messages in thread