On Fri, Aug 22, 2025 at 11:17 PM Jesse Taube jesse@rivosinc.com wrote:
Various parts of the kernel decode and read instruction from memory. Functions like get_insn, GET_INSN_LENGTH and riscv_insn_is_c are defined in multiple places. Consolidate these functions into the insn.h and the newly added insn.c.
Signed-off-by: Jesse Taube jesse@rivosinc.com
RFC -> V1:
- No change
V2 -> V1:
- No change
arch/riscv/include/asm/bug.h | 12 --- arch/riscv/include/asm/insn.h | 131 ++++++++++++++++++++++- arch/riscv/kernel/Makefile | 1 + arch/riscv/kernel/insn.c | 151 +++++++++++++++++++++++++++ arch/riscv/kernel/kgdb.c | 102 +----------------- arch/riscv/kernel/probes/kprobes.c | 1 + arch/riscv/kernel/traps.c | 5 +- arch/riscv/kernel/traps_misaligned.c | 93 ++++------------- 8 files changed, 309 insertions(+), 187 deletions(-) create mode 100644 arch/riscv/kernel/insn.c
diff --git a/arch/riscv/include/asm/bug.h b/arch/riscv/include/asm/bug.h index 1aaea81fb141..a2777eb67ad1 100644 --- a/arch/riscv/include/asm/bug.h +++ b/arch/riscv/include/asm/bug.h @@ -12,21 +12,9 @@
#include <asm/asm.h>
-#define __INSN_LENGTH_MASK _UL(0x3) -#define __INSN_LENGTH_32 _UL(0x3) -#define __COMPRESSED_INSN_MASK _UL(0xffff)
#define __BUG_INSN_32 _UL(0x00100073) /* ebreak */ #define __BUG_INSN_16 _UL(0x9002) /* c.ebreak */
-#define GET_INSN_LENGTH(insn) \ -({ \
unsigned long __len; \__len = ((insn & __INSN_LENGTH_MASK) == __INSN_LENGTH_32) ? \4UL : 2UL; \__len; \-})
typedef u32 bug_insn_t;
#ifdef CONFIG_GENERIC_BUG_RELATIVE_POINTERS diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h index 09fde95a5e8f..ba74e5b8262c 100644 --- a/arch/riscv/include/asm/insn.h +++ b/arch/riscv/include/asm/insn.h @@ -64,6 +64,7 @@ #define RVG_RS2_OPOFF 20 #define RVG_RD_OPOFF 7 #define RVG_RS1_MASK GENMASK(4, 0) +#define RVG_RS2_MASK GENMASK(4, 0) #define RVG_RD_MASK GENMASK(4, 0)
/* The bit field of immediate value in RVC J instruction */ @@ -121,17 +122,27 @@ #define RVC_C0_RS1_OPOFF 7 #define RVC_C0_RS2_OPOFF 2 #define RVC_C0_RD_OPOFF 2 +#define RVC_C0_RS1_MASK GENMASK(2, 0) +#define RVC_C0_RS2_MASK GENMASK(2, 0) +#define RVC_C0_RD_MASK GENMASK(2, 0) +#define RVC_C0_REG_OFFSET 8
/* The register offset in RVC op=C1 instruction */ #define RVC_C1_RS1_OPOFF 7 #define RVC_C1_RS2_OPOFF 2 #define RVC_C1_RD_OPOFF 7 +#define RVC_C1_RS1_MASK GENMASK(2, 0) +#define RVC_C1_RS2_MASK GENMASK(2, 0) +#define RVC_C1_RD_MASK GENMASK(2, 0) +#define RVC_C1_REG_OFFSET 8
/* The register offset in RVC op=C2 instruction */ #define RVC_C2_RS1_OPOFF 7 #define RVC_C2_RS2_OPOFF 2 #define RVC_C2_RD_OPOFF 7 #define RVC_C2_RS1_MASK GENMASK(4, 0) +#define RVC_C2_RS2_MASK GENMASK(4, 0) +#define RVC_C2_RD_MASK GENMASK(4, 0)
/* parts of opcode for RVG*/ #define RVG_OPCODE_FENCE 0x0f @@ -226,12 +237,26 @@ #define RVC_MASK_C_EBREAK 0xffff #define RVG_MASK_EBREAK 0xffffffff #define RVG_MASK_SRET 0xffffffff +#define RVC_MASK_C GENMASK(15, 0)
#define __INSN_LENGTH_MASK _UL(0x3) #define __INSN_LENGTH_GE_32 _UL(0x3) #define __INSN_OPCODE_MASK _UL(0x7F) #define __INSN_BRANCH_OPCODE _UL(RVG_OPCODE_BRANCH)
+#define GET_INSN_LENGTH(insn) \ +({ \
unsigned long __len; \__len = ((insn & __INSN_LENGTH_MASK) == __INSN_LENGTH_GE_32) ? \4UL : 2UL; \__len; \+})
+static __always_inline bool riscv_insn_is_c(u32 code) +{
return (code & (__INSN_LENGTH_MASK)) != (__INSN_LENGTH_GE_32);+}
#define __RISCV_INSN_FUNCS(name, mask, val) \ static __always_inline bool riscv_insn_is_##name(u32 code) \ { \ @@ -260,7 +285,7 @@ __RISCV_INSN_FUNCS(c_bnez, RVC_MASK_C_BNEZ, RVC_MATCH_C_BNEZ) __RISCV_INSN_FUNCS(c_ebreak, RVC_MASK_C_EBREAK, RVC_MATCH_C_EBREAK) __RISCV_INSN_FUNCS(ebreak, RVG_MASK_EBREAK, RVG_MATCH_EBREAK) __RISCV_INSN_FUNCS(sret, RVG_MASK_SRET, RVG_MATCH_SRET) -__RISCV_INSN_FUNCS(fence, RVG_MASK_FENCE, RVG_MATCH_FENCE); +__RISCV_INSN_FUNCS(fence, RVG_MASK_FENCE, RVG_MATCH_FENCE)
/* special case to catch _any_ system instruction */ static __always_inline bool riscv_insn_is_system(u32 code) @@ -295,6 +320,10 @@ static __always_inline bool riscv_insn_is_c_jalr(u32 code) ({typeof(x) x_ = (x); \ (RV_X(x_, RVG_RS1_OPOFF, RVG_RS1_MASK)); })
+#define RV_EXTRACT_RS2_REG(x) \
({typeof(x) x_ = (x); \(RV_X(x_, RVG_RS2_OPOFF, RVG_RS2_MASK)); })#define RV_EXTRACT_RD_REG(x) \ ({typeof(x) x_ = (x); \ (RV_X(x_, RVG_RD_OPOFF, RVG_RD_MASK)); }) @@ -322,9 +351,41 @@ static __always_inline bool riscv_insn_is_c_jalr(u32 code) (RV_X(x_, RV_B_IMM_11_OPOFF, RV_B_IMM_11_MASK) << RV_B_IMM_11_OFF) | \ (RV_IMM_SIGN(x_) << RV_B_IMM_SIGN_OFF); })
+#define RVC_EXTRACT_C0_RS1_REG(x) \
({typeof(x) x_ = (x); \(RVC_X(x_, RVC_C0_RS1_OPOFF, RVC_C0_RS1_MASK)); })+#define RVC_EXTRACT_C0_RS2_REG(x) \
({typeof(x) x_ = (x); \(RVC_X(x_, RVC_C0_RS2_OPOFF, RVC_C0_RS2_MASK)); })+#define RVC_EXTRACT_C0_RD_REG(x) \
({typeof(x) x_ = (x); \(RVC_X(x_, RVC_C0_RD_OPOFF, RVC_C0_RD_MASK)); })+#define RVC_EXTRACT_C1_RS1_REG(x) \
({typeof(x) x_ = (x); \(RVC_X(x_, RVC_C1_RS1_OPOFF, RVC_C1_RS1_MASK)); })+#define RVC_EXTRACT_C1_RS2_REG(x) \
({typeof(x) x_ = (x); \(RVC_X(x_, RVC_C1_RS2_OPOFF, RVC_C1_RS2_MASK)); })+#define RVC_EXTRACT_C1_RD_REG(x) \
({typeof(x) x_ = (x); \(RVC_X(x_, RVC_C1_RD_OPOFF, RVC_C1_RD_MASK)); })#define RVC_EXTRACT_C2_RS1_REG(x) \ ({typeof(x) x_ = (x); \
(RV_X(x_, RVC_C2_RS1_OPOFF, RVC_C2_RS1_MASK)); })
(RVC_X(x_, RVC_C2_RS1_OPOFF, RVC_C2_RS1_MASK)); })+#define RVC_EXTRACT_C2_RS2_REG(x) \
({typeof(x) x_ = (x); \(RVC_X(x_, RVC_C2_RS2_OPOFF, RVC_C2_RS2_MASK)); })+#define RVC_EXTRACT_C2_RD_REG(x) \
({typeof(x) x_ = (x); \(RVC_X(x_, RVC_C2_RD_OPOFF, RVC_C2_RD_MASK)); })#define RVC_EXTRACT_JTYPE_IMM(x) \ ({typeof(x) x_ = (x); \ @@ -354,6 +415,66 @@ static __always_inline bool riscv_insn_is_c_jalr(u32 code)
#define RVV_EXRACT_VL_VS_WIDTH(x) RVFDQ_EXTRACT_FL_FS_WIDTH(x)
+/*
- Get the rs1 register number from RV or RVC instruction.
- @insn: instruction to process
- Return: rs1 register
- */
+static inline unsigned int riscv_insn_extract_rs1_reg(u32 insn) +{
switch (RVC_INSN_OPCODE_MASK & insn) {case RVC_OPCODE_C0:return RVC_EXTRACT_C0_RS1_REG(insn) + RVC_C0_REG_OFFSET;case RVC_OPCODE_C1:return RVC_EXTRACT_C1_RS1_REG(insn) + RVC_C1_REG_OFFSET;case RVC_OPCODE_C2:return RVC_EXTRACT_C2_RS1_REG(insn);default:return RV_EXTRACT_RS1_REG(insn);}+}
+/*
- Get the rs2 register number from RV or RVC instruction.
- @insn: instruction to process
- Return: rs2 register
- */
+static inline unsigned int riscv_insn_extract_rs2_reg(u32 insn) +{
switch (RVC_INSN_OPCODE_MASK & insn) {case RVC_OPCODE_C0:return RVC_EXTRACT_C0_RS2_REG(insn) + RVC_C0_REG_OFFSET;case RVC_OPCODE_C1:return RVC_EXTRACT_C1_RS2_REG(insn) + RVC_C1_REG_OFFSET;case RVC_OPCODE_C2:return RVC_EXTRACT_C2_RS2_REG(insn);default:return RV_EXTRACT_RS2_REG(insn);}+}
+/*
- Get the rd register number from RV or RVC instruction.
- @insn: instruction to process
- Return: rd register
- */
+static inline unsigned int riscv_insn_extract_rd_reg(u32 insn) +{
switch (RVC_INSN_OPCODE_MASK & insn) {case RVC_OPCODE_C0:return RVC_EXTRACT_C0_RD_REG(insn) + RVC_C0_REG_OFFSET;case RVC_OPCODE_C1:return RVC_EXTRACT_C1_RD_REG(insn) + RVC_C1_REG_OFFSET;case RVC_OPCODE_C2:return RVC_EXTRACT_C2_RD_REG(insn);default:return RV_EXTRACT_RD_REG(insn);}+}
/*
- Get the immediate from a J-type instruction.
@@ -428,4 +549,10 @@ static inline void riscv_insn_insert_utype_itype_imm(u32 *utype_insn, u32 *itype *utype_insn |= (imm & RV_U_IMM_31_12_MASK) + ((imm & BIT(11)) << 1); *itype_insn |= ((imm & RV_I_IMM_11_0_MASK) << RV_I_IMM_11_0_OPOFF); }
+#include <asm/ptrace.h>
+int get_insn(struct pt_regs *regs, ulong epc, ulong *r_insn); +unsigned long get_step_address(struct pt_regs *regs, u32 code);
#endif /* _ASM_RISCV_INSN_H */ diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile index f7480c9c6f8d..4f719b09e5ad 100644 --- a/arch/riscv/kernel/Makefile +++ b/arch/riscv/kernel/Makefile @@ -51,6 +51,7 @@ obj-$(CONFIG_RISCV_ALTERNATIVE) += alternative.o obj-y += cpu.o obj-y += cpufeature.o obj-y += entry.o +obj-y += insn.o obj-y += irq.o obj-y += process.o obj-y += ptrace.o diff --git a/arch/riscv/kernel/insn.c b/arch/riscv/kernel/insn.c new file mode 100644 index 000000000000..dd2a6ef9fd25 --- /dev/null +++ b/arch/riscv/kernel/insn.c @@ -0,0 +1,151 @@ +// SPDX-License-Identifier: GPL-2.0-only +/*
- Copyright 2025 Rivos, Inc
- */
+#include <asm/insn.h> +#include <asm/ptrace.h> +#include <asm/uaccess.h>
+#define __read_insn(regs, insn, insn_addr, type) \ +({ \
int __ret; \\if (user_mode(regs)) { \__ret = get_user(insn, (type __user *) insn_addr); \} else { \insn = *(type *)insn_addr; \__ret = 0; \} \\__ret; \+})
+/*
- Update a set of two instructions (U-type + I-type) with an immediate value.
- Used for example in auipc+jalrs pairs the U-type instructions contains
- a 20bit upper immediate representing bits[31:12], while the I-type
- instruction contains a 12bit immediate representing bits[11:0].
- This also takes into account that both separate immediates are
- considered as signed values, so if the I-type immediate becomes
- negative (BIT(11) set) the U-type part gets adjusted.
- @regs: pointer to the utype instruction of the pair
- @epc: pointer to the itype instruction of the pair
- @r_insn: the immediate to insert into the two instructions
- Return: combined immediate
- */
+int get_insn(struct pt_regs *regs, ulong epc, ulong *r_insn) +{
ulong insn = 0;if (epc & 0x2) {ulong tmp = 0;if (__read_insn(regs, insn, epc, u16))return -EFAULT;/* __get_user() uses regular "lw" which sign extend the loaded* value make sure to clear higher order bits in case we "or" it* below with the upper 16 bits half.*/insn &= RVC_MASK_C;if (riscv_insn_is_c(insn)) {*r_insn = insn;return 0;}epc += sizeof(u16);if (__read_insn(regs, tmp, epc, u16))return -EFAULT;*r_insn = (tmp << 16) | insn;return 0;} else {if (__read_insn(regs, insn, epc, u32))return -EFAULT;if (!riscv_insn_is_c(insn)) {*r_insn = insn;return 0;}insn &= RVC_MASK_C;*r_insn = insn;return 0;}+}
+/* Calculate the new address for after a step */ +unsigned long get_step_address(struct pt_regs *regs, u32 code) +{
unsigned long pc = regs->epc;unsigned int rs1_num, rs2_num;if ((code & __INSN_LENGTH_MASK) != __INSN_LENGTH_GE_32) {if (riscv_insn_is_c_jalr(code) ||riscv_insn_is_c_jr(code)) {rs1_num = riscv_insn_extract_rs1_reg(code);return regs_get_register(regs, rs1_num);} else if (riscv_insn_is_c_j(code) ||riscv_insn_is_c_jal(code)) {return RVC_EXTRACT_JTYPE_IMM(code) + pc;} else if (riscv_insn_is_c_beqz(code)) {rs1_num = riscv_insn_extract_rs1_reg(code);if (!rs1_num || regs_get_register(regs, rs1_num) == 0)return RVC_EXTRACT_BTYPE_IMM(code) + pc;elsereturn pc + 2;} else if (riscv_insn_is_c_bnez(code)) {rs1_num = riscv_insn_extract_rs1_reg(RVC_C1_RS1_OPOFF);if (rs1_num && regs_get_register(regs, rs1_num) != 0)return RVC_EXTRACT_BTYPE_IMM(code) + pc;elsereturn pc + 2;} else {return pc + 2;}} else {if ((code & __INSN_OPCODE_MASK) == __INSN_BRANCH_OPCODE) {bool result = false;long imm = RV_EXTRACT_BTYPE_IMM(code);unsigned long rs1_val = 0, rs2_val = 0;rs1_num = riscv_insn_extract_rs1_reg(code);rs2_num = riscv_insn_extract_rs2_reg(code);if (rs1_num)rs1_val = regs_get_register(regs, rs1_num);if (rs2_num)rs2_val = regs_get_register(regs, rs2_num);if (riscv_insn_is_beq(code))result = (rs1_val == rs2_val) ? true : false;else if (riscv_insn_is_bne(code))result = (rs1_val != rs2_val) ? true : false;else if (riscv_insn_is_blt(code))result =((long)rs1_val <(long)rs2_val) ? true : false;else if (riscv_insn_is_bge(code))result =((long)rs1_val >=(long)rs2_val) ? true : false;else if (riscv_insn_is_bltu(code))result = (rs1_val < rs2_val) ? true : false;else if (riscv_insn_is_bgeu(code))result = (rs1_val >= rs2_val) ? true : false;if (result)return imm + pc;elsereturn pc + 4;} else if (riscv_insn_is_jal(code)) {return RV_EXTRACT_JTYPE_IMM(code) + pc;} else if (riscv_insn_is_jalr(code)) {rs1_num = riscv_insn_extract_rs1_reg(code);return RV_EXTRACT_ITYPE_IMM(code) +(rs1_num ? regs_get_register(regs, rs1_num) : 0);} else if (riscv_insn_is_sret(code)) {return pc;} else {return pc + 4;}}+} diff --git a/arch/riscv/kernel/kgdb.c b/arch/riscv/kernel/kgdb.c index 9f3db3503dab..aafc1424fc81 100644 --- a/arch/riscv/kernel/kgdb.c +++ b/arch/riscv/kernel/kgdb.c @@ -23,111 +23,19 @@ enum { static unsigned long stepped_address; static unsigned int stepped_opcode;
-static int decode_register_index(unsigned long opcode, int offset) -{
return (opcode >> offset) & 0x1F;-}
-static int decode_register_index_short(unsigned long opcode, int offset) -{
return ((opcode >> offset) & 0x7) + 8;-}
-/* Calculate the new address for after a step */ -static int get_step_address(struct pt_regs *regs, unsigned long *next_addr) -{
unsigned long pc = regs->epc;unsigned long *regs_ptr = (unsigned long *)regs;unsigned int rs1_num, rs2_num;int op_code;if (get_kernel_nofault(op_code, (void *)pc))return -EINVAL;if ((op_code & __INSN_LENGTH_MASK) != __INSN_LENGTH_GE_32) {if (riscv_insn_is_c_jalr(op_code) ||riscv_insn_is_c_jr(op_code)) {rs1_num = decode_register_index(op_code, RVC_C2_RS1_OPOFF);*next_addr = regs_ptr[rs1_num];} else if (riscv_insn_is_c_j(op_code) ||riscv_insn_is_c_jal(op_code)) {*next_addr = RVC_EXTRACT_JTYPE_IMM(op_code) + pc;} else if (riscv_insn_is_c_beqz(op_code)) {rs1_num = decode_register_index_short(op_code,RVC_C1_RS1_OPOFF);if (!rs1_num || regs_ptr[rs1_num] == 0)*next_addr = RVC_EXTRACT_BTYPE_IMM(op_code) + pc;else*next_addr = pc + 2;} else if (riscv_insn_is_c_bnez(op_code)) {rs1_num =decode_register_index_short(op_code, RVC_C1_RS1_OPOFF);if (rs1_num && regs_ptr[rs1_num] != 0)*next_addr = RVC_EXTRACT_BTYPE_IMM(op_code) + pc;else*next_addr = pc + 2;} else {*next_addr = pc + 2;}} else {if ((op_code & __INSN_OPCODE_MASK) == __INSN_BRANCH_OPCODE) {bool result = false;long imm = RV_EXTRACT_BTYPE_IMM(op_code);unsigned long rs1_val = 0, rs2_val = 0;rs1_num = decode_register_index(op_code, RVG_RS1_OPOFF);rs2_num = decode_register_index(op_code, RVG_RS2_OPOFF);if (rs1_num)rs1_val = regs_ptr[rs1_num];if (rs2_num)rs2_val = regs_ptr[rs2_num];if (riscv_insn_is_beq(op_code))result = (rs1_val == rs2_val) ? true : false;else if (riscv_insn_is_bne(op_code))result = (rs1_val != rs2_val) ? true : false;else if (riscv_insn_is_blt(op_code))result =((long)rs1_val <(long)rs2_val) ? true : false;else if (riscv_insn_is_bge(op_code))result =((long)rs1_val >=(long)rs2_val) ? true : false;else if (riscv_insn_is_bltu(op_code))result = (rs1_val < rs2_val) ? true : false;else if (riscv_insn_is_bgeu(op_code))result = (rs1_val >= rs2_val) ? true : false;if (result)*next_addr = imm + pc;else*next_addr = pc + 4;} else if (riscv_insn_is_jal(op_code)) {*next_addr = RV_EXTRACT_JTYPE_IMM(op_code) + pc;} else if (riscv_insn_is_jalr(op_code)) {rs1_num = decode_register_index(op_code, RVG_RS1_OPOFF);if (rs1_num)*next_addr = ((unsigned long *)regs)[rs1_num];*next_addr += RV_EXTRACT_ITYPE_IMM(op_code);} else if (riscv_insn_is_sret(op_code)) {*next_addr = pc;} else {*next_addr = pc + 4;}}return 0;-}
static int do_single_step(struct pt_regs *regs) { /* Determine where the target instruction will send us to */
unsigned long addr = 0;int error = get_step_address(regs, &addr);
unsigned long addr, insn;int error = get_insn(regs, regs->epc, &insn); if (error) return error;addr = get_step_address(regs, insn);/* Store the op code in the stepped address */
error = get_kernel_nofault(stepped_opcode, (void *)addr);
error = get_insn(regs, addr, stepped_opcode);
This line gives following compile error:
arch/riscv/kernel/kgdb.c: In function 'do_single_step': arch/riscv/kernel/kgdb.c:38:38: error: passing argument 3 of 'get_insn' makes pointer from integer without a cast [-Wint-conversion] 38 | error = get_insn(regs, addr, stepped_opcode); | ^~~~~~~~~~~~~~ | | | unsigned int In file included from arch/riscv/kernel/kgdb.c:14: ./arch/riscv/include/asm/insn.h:555:54: note: expected 'ulong *' {aka 'long unsigned int *'} but argument is of type 'unsigned int' 555 | int get_insn(struct pt_regs *regs, ulong epc, ulong *r_insn); | ~~~~~~~^~~~~~
If you are touching some source then at least compile test it.
Regards, Anup