Re: [PATCH v2 06/13] RISC-V: kprobes: use central defined funct3 constants

Conor Dooley
2022-11-29T23:22:29Z

On Mon, Nov 28, 2022 at 11:26:25AM +0100, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>
> Don't redefine values that are already available in a central header.

Which central header?
Being intentionally dense, but it'd be nice to mention it since it's not
in the diffstat?

> Use the central ones instead.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
> arch/riscv/kernel/probes/simulate-insn.c | 19 ++++++-------------
> 1 file changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/arch/riscv/kernel/probes/simulate-insn.c b/arch/riscv/kernel/probes/simulate-insn.c
> index d73e96f6ed7c..330afe9331a8 100644
> --- a/arch/riscv/kernel/probes/simulate-insn.c
> +++ b/arch/riscv/kernel/probes/simulate-insn.c
> @@ -136,13 +136,6 @@ bool __kprobes simulate_auipc(u32 opcode, unsigned long addr, struct pt_regs *re
> #define branch_offset(opcode) \
> sign_extend32((branch_imm(opcode)), 12)
>
> -#define BRANCH_BEQ 0x0
> -#define BRANCH_BNE 0x1
> -#define BRANCH_BLT 0x4
> -#define BRANCH_BGE 0x5
> -#define BRANCH_BLTU 0x6
> -#define BRANCH_BGEU 0x7
> -
> bool __kprobes simulate_branch(u32 opcode, unsigned long addr, struct pt_regs *regs)
> {
> /*
> @@ -169,22 +162,22 @@ bool __kprobes simulate_branch(u32 opcode, unsigned long addr, struct pt_regs *r
>
> offset_tmp = branch_offset(opcode);
> switch (branch_funct3(opcode)) {
> - case BRANCH_BEQ:
> + case RVG_FUNCT3_BEQ:
> offset = (rs1_val == rs2_val) ? offset_tmp : 4;
> break;
> - case BRANCH_BNE:
> + case RVG_FUNCT3_BNE:
> offset = (rs1_val != rs2_val) ? offset_tmp : 4;
> break;
> - case BRANCH_BLT:
> + case RVG_FUNCT3_BLT:
> offset = ((long)rs1_val < (long)rs2_val) ? offset_tmp : 4;
> break;
> - case BRANCH_BGE:
> + case RVG_FUNCT3_BGE:
> offset = ((long)rs1_val >= (long)rs2_val) ? offset_tmp : 4;
> break;
> - case BRANCH_BLTU:
> + case RVG_FUNCT3_BLTU:
> offset = (rs1_val < rs2_val) ? offset_tmp : 4;
> break;
> - case BRANCH_BGEU:
> + case RVG_FUNCT3_BGEU:
> offset = (rs1_val >= rs2_val) ? offset_tmp : 4;
> break;
> default:

Still suffering from the carried-in build issues, but this change also
looks grand in isolation?

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>