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

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

[Xiaofeng Yuan]

The c.jal instruction is currently marked REJECTED in kprobes
instruction decoding, but it should be SIMULATED like other
compressed jump instructions.

Add simulate_c_jal() which saves the return address to x1 (ra)
and sets the program counter to the target offset, reusing
simulate_c_j for the common jump logic.

Signed-off-by: Xiaofeng Yuan <xiaofengmian@163.com>
---
v2: reuse simulate_c_j() for common jump logic (per Nam Cao's review) (resend: v2 had wrong diff)

 arch/riscv/kernel/probes/decode-insn.c   | 2 +-
 arch/riscv/kernel/probes/simulate-insn.c | 8 ++++++++
 arch/riscv/kernel/probes/simulate-insn.h | 1 +
 3 files changed, 10 insertions(+), 1 deletion(-)

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);
 
 	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;
+
+	return simulate_c_j(opcode, addr, regs);
+}
+
 static bool __kprobes simulate_c_jr_jalr(u32 opcode, unsigned long addr, struct pt_regs *regs,
 					 bool is_jalr)
 {
diff --git a/arch/riscv/kernel/probes/simulate-insn.h b/arch/riscv/kernel/probes/simulate-insn.h
index 44ebbc444..b89e1bb01 100644
--- a/arch/riscv/kernel/probes/simulate-insn.h
+++ b/arch/riscv/kernel/probes/simulate-insn.h
@@ -25,6 +25,7 @@ bool simulate_branch(u32 opcode, unsigned long addr, struct pt_regs *regs);
 bool simulate_jal(u32 opcode, unsigned long addr, struct pt_regs *regs);
 bool simulate_jalr(u32 opcode, unsigned long addr, struct pt_regs *regs);
 bool simulate_c_j(u32 opcode, unsigned long addr, struct pt_regs *regs);
+bool simulate_c_jal(u32 opcode, unsigned long addr, struct pt_regs *regs);
 bool simulate_c_jr(u32 opcode, unsigned long addr, struct pt_regs *regs);
 bool simulate_c_jalr(u32 opcode, unsigned long addr, struct pt_regs *regs);
 bool simulate_c_bnez(u32 opcode, unsigned long addr, struct pt_regs *regs);
-- 
2.43.0


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

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

[Xiaofeng Yuan]

The c.jal instruction is currently marked REJECTED in kprobes
instruction decoding, but it should be SIMULATED like other
compressed jump instructions.

Add simulate_c_jal() which saves the return address to x1 (ra)
and sets the program counter to the target offset, reusing
simulate_c_j for the common jump logic.

Signed-off-by: Xiaofeng Yuan <xiaofengmian@163.com>
---
v2: reuse simulate_c_j() for common jump logic (per Nam Cao's review) (resend: v2 had wrong diff)

 arch/riscv/kernel/probes/decode-insn.c   | 2 +-
 arch/riscv/kernel/probes/simulate-insn.c | 8 ++++++++
 arch/riscv/kernel/probes/simulate-insn.h | 1 +
 3 files changed, 10 insertions(+), 1 deletion(-)

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);
 
 	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;
+
+	return simulate_c_j(opcode, addr, regs);
+}
+
 static bool __kprobes simulate_c_jr_jalr(u32 opcode, unsigned long addr, struct pt_regs *regs,
 					 bool is_jalr)
 {
diff --git a/arch/riscv/kernel/probes/simulate-insn.h b/arch/riscv/kernel/probes/simulate-insn.h
index 44ebbc444..b89e1bb01 100644
--- a/arch/riscv/kernel/probes/simulate-insn.h
+++ b/arch/riscv/kernel/probes/simulate-insn.h
@@ -25,6 +25,7 @@ bool simulate_branch(u32 opcode, unsigned long addr, struct pt_regs *regs);
 bool simulate_jal(u32 opcode, unsigned long addr, struct pt_regs *regs);
 bool simulate_jalr(u32 opcode, unsigned long addr, struct pt_regs *regs);
 bool simulate_c_j(u32 opcode, unsigned long addr, struct pt_regs *regs);
+bool simulate_c_jal(u32 opcode, unsigned long addr, struct pt_regs *regs);
 bool simulate_c_jr(u32 opcode, unsigned long addr, struct pt_regs *regs);
 bool simulate_c_jalr(u32 opcode, unsigned long addr, struct pt_regs *regs);
 bool simulate_c_bnez(u32 opcode, unsigned long addr, struct pt_regs *regs);
-- 
2.43.0

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

[Nam Cao]

Xiaofeng Yuan <xiaofengmian@163.com> 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

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

[Nam Cao]

Xiaofeng Yuan <xiaofengmian@163.com> 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

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