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

From: Conor Dooley <conor@kernel.org>
To: Heiko Stuebner <heiko@sntech.de>
Cc: linux-riscv@lists.infradead.org, palmer@dabbelt.com,
	christoph.muellner@vrull.eu, prabhakar.csengg@gmail.com,
	philipp.tomsich@vrull.eu, ajones@ventanamicro.com,
	emil.renner.berthing@canonical.com,
	Heiko Stuebner <heiko.stuebner@vrull.eu>
Subject: Re: [PATCH v2 06/13] RISC-V: kprobes: use central defined funct3 constants
Date: Tue, 29 Nov 2022 23:22:10 +0000	[thread overview]
Message-ID: <Y4aUIsKbJ//CG9iV@spud> (raw)
In-Reply-To: <20221128102632.435174-7-heiko@sntech.de>

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>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv