Re: [PATCH v3] riscv: probes: simulate c.jal instruction

[Nam Cao]

Xiaofeng Yuan <xiaofengmian@xxxxxxx> writes:
> diff --git a/arch/riscv/kernel/probes/decode-insn.c b/arch/riscv/kernel/probes/decode-insn.c
> index 65d9590bf..8506470e9 100644
> --- a/arch/riscv/kernel/probes/decode-insn.c
> +++ b/arch/riscv/kernel/probes/decode-insn.c
> @@ -29,7 +29,7 @@ riscv_probe_decode_insn(probe_opcode_t *addr, struct arch_probe_insn *api)
>  	 * TODO: the REJECTED ones below need to be implemented
>  	 */
>  #ifdef CONFIG_RISCV_ISA_C
> -	RISCV_INSN_REJECTED(c_jal,		insn);
> +	RISCV_INSN_SET_SIMULATE(c_jal,		insn);
>  	RISCV_INSN_REJECTED(c_ebreak,		insn);

Nit: move this RISCV_INSN_SET_SIMULATE down to be with the others.

>  	RISCV_INSN_SET_SIMULATE(c_j,		insn);
> diff --git a/arch/riscv/kernel/probes/simulate-insn.c b/arch/riscv/kernel/probes/simulate-insn.c
> index fa581590c..6d7a5f949 100644
> --- a/arch/riscv/kernel/probes/simulate-insn.c
> +++ b/arch/riscv/kernel/probes/simulate-insn.c
> @@ -163,6 +163,14 @@ bool __kprobes simulate_c_j(u32 opcode, unsigned long addr, struct pt_regs *regs
>  	return true;
>  }
>  
> +bool __kprobes simulate_c_jal(u32 opcode, unsigned long addr, struct pt_regs *regs)
> +{
> +	if (!rv_insn_reg_set_val(regs, 1, addr + 2))
> +		return false;

rv_insn_reg_set_val() is a helper to change a register by index. But
it's always RA in this case, so I think it's much clearer to simply:

	regs->ra = addr + 2;

(with the two above suggested changes, the patch would look identical to
a patch I have but I forgot to send, and I would be able to give my
Tested-by tag without additional effort from me :D)

Nam