Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 22 Jul 2021 10:26:59 -0700 (PDT)
Subject:     Re: [PATCH -next 1/2] riscv: implemented auipc simulate instruction
In-Reply-To: <20210629023455.280998-1-chenlifu@huawei.com>
CC:     Paul Walmsley <paul.walmsley@sifive.com>, aou@eecs.berkeley.edu,
        guoren@linux.alibaba.com, chenlifu@huawei.com, penberg@kernel.org,
        mhiramat@kernel.org, me@packi.ch, linux-riscv@lists.infradead.org,
        linux-kernel@vger.kernel.org
From:   Palmer Dabbelt <palmer@dabbelt.com>
To:     chenlifu@huawei.com
Message-ID: <mhng-65ffe489-280a-4ebd-94e3-745f7114218d@palmerdabbelt-glaptop>
Mime-Version: 1.0 (MHng)
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Mon, 28 Jun 2021 19:34:54 PDT (-0700), chenlifu@huawei.com wrote:
> To test the kprobe-based event tracing, we prepare
> a kernel module 'kprobe_test.ko' to add the probes.
> The assembly codes (partially) of the module are as follows:
> ...
> 0000000000000000 <kprobe_test_branch>:
> ...
> 0000000000000038 <.LVL1>:
>   38:	00000597          	auipc	a1,0x0
>   3c:	00058593          	mv	a1,a1
> ...
>
> Test the kprobe-based event tracing in qemu-system-riscv64:
> First, install the kprobe test module:
> insmod /root/kprobe_test.ko
>
> Then, add a probe as a new event at an 'auipc' instruction,
> the following error occurs due to the instruction not allowed to probe yet:
> echo "p:auipc kprobe_test:kprobe_test_branch+0x38 epc=%epc opcode=+0(%epc):x32" >> /sys/kernel/debug/tracing/kprobe_events
> sh: write error: Invalid argument
>
> This patch implemented the 'auipc' simulate instruction and allowed to probe it.
> Merge this patch and perform the test again, the test results are as follows:
> First, add a probe at the 'auipc' instruction:
> echo "p:auipc kprobe_test:kprobe_test_branch+0x38 epc=%epc opcode=+0(%epc):x32" >> /sys/kernel/debug/tracing/kprobe_events
> echo 1 > /sys/kernel/debug/tracing/events/kprobes/auipc/enable
>
> Then, do something to run to the probe.
> After that, see the traced information:
> cat /sys/kernel/debug/tracing/trace
> sysctl-58      [001] d...   179.126350: auipc: (kprobe_test_branch+0x38/0x10e [kprobe_test]) epc=0xffffffff016122aa opcode=0x100073
>
> Now we can see the traced information.
> The actual address of the symbol 'kprobe_test_branch' is as follows:
> cat /proc/kallsyms | grep kprobe_test_branch
> ffffffff01612272 t kprobe_test_branch   [kprobe_test]
>
> Based on the traced information and the actual address of the symbol
> 'kprobe_test_branch', we can also see that the 'auipc' instruction
> has been replaced by 'ebreak(0x100073)' instruction.
>
> --------
>
> Signed-off-by: Chen Lifu <chenlifu@huawei.com>
> ---
>  arch/riscv/kernel/probes/decode-insn.c   |  2 +-
>  arch/riscv/kernel/probes/simulate-insn.c | 34 ++++++++++++++++++++++++
>  2 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/probes/decode-insn.c b/arch/riscv/kernel/probes/decode-insn.c
> index 0ed043acc882..5eb03fb61450 100644
> --- a/arch/riscv/kernel/probes/decode-insn.c
> +++ b/arch/riscv/kernel/probes/decode-insn.c
> @@ -38,11 +38,11 @@ riscv_probe_decode_insn(probe_opcode_t *addr, struct arch_probe_insn *api)
>  	RISCV_INSN_REJECTED(c_ebreak,		insn);
>  #endif
>
> -	RISCV_INSN_REJECTED(auipc,		insn);
>  	RISCV_INSN_REJECTED(branch,		insn);
>
>  	RISCV_INSN_SET_SIMULATE(jal,		insn);
>  	RISCV_INSN_SET_SIMULATE(jalr,		insn);
> +	RISCV_INSN_SET_SIMULATE(auipc,		insn);
>
>  	return INSN_GOOD;
>  }
> diff --git a/arch/riscv/kernel/probes/simulate-insn.c b/arch/riscv/kernel/probes/simulate-insn.c
> index 2519ce26377d..b81719522d5c 100644
> --- a/arch/riscv/kernel/probes/simulate-insn.c
> +++ b/arch/riscv/kernel/probes/simulate-insn.c
> @@ -83,3 +83,37 @@ bool __kprobes simulate_jalr(u32 opcode, unsigned long addr, struct pt_regs *reg
>
>  	return ret;
>  }
> +
> +#define auipc_rd_idx(opcode) \
> +	((opcode >> 7) & 0x1f)
> +
> +#define auipc_imm(opcode) \
> +	((((opcode) >> 12) & 0xfffff) << 12)
> +
> +#if __riscv_xlen == 64
> +#define auipc_offset(opcode)	sign_extend64(auipc_imm(opcode), 31)
> +#elif __riscv_xlen == 32
> +#define auipc_offset(opcode)	auipc_imm(opcode)
> +#else
> +#error "Unexpected __riscv_xlen"
> +#endif
> +
> +bool __kprobes simulate_auipc(u32 opcode, unsigned long addr, struct pt_regs *regs)
> +{
> +	/*
> +	 * auipc instruction:
> +	 *  31        12 11 7 6      0
> +	 * | imm[31:12] | rd | opcode |
> +	 *        20       5     7
> +	 */
> +
> +	u32 rd_idx = auipc_rd_idx(opcode);
> +	unsigned long rd_val = addr + auipc_offset(opcode);
> +
> +	if (!rv_insn_reg_set_val(regs, rd_idx, rd_val))
> +		return false;
> +
> +	instruction_pointer_set(regs, addr + 4);
> +
> +	return true;
> +}

Thanks.  These are on for-next, with the checkpatch errors fixed and the 
commit messages mostly removed -- it was all pretty awkwardly phrased so 
I didn't want to clean it up.