Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <a00f788b-3799-4dcd-0401-846159e151c4@huawei.com>
Date:   Wed, 4 Jan 2023 16:35:15 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH v5 6/9] riscv/kprobe: Add code to check if kprobe can be
 optimized
To:     =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= <bjorn@kernel.org>,
        Chen Guokai <chenguokai17@mails.ucas.ac.cn>,
        <paul.walmsley@sifive.com>, <palmer@dabbelt.com>,
        <aou@eecs.berkeley.edu>, <rostedt@goodmis.org>, <mingo@redhat.com>,
        <sfr@canb.auug.org.au>
CC:     <linux-riscv@lists.infradead.org>, <linux-kernel@vger.kernel.org>
References: <20221224114315.850130-1-chenguokai17@mails.ucas.ac.cn>
 <20221224114315.850130-7-chenguokai17@mails.ucas.ac.cn>
 <87sfgsvmnj.fsf@all.your.base.are.belong.to.us>
From:   "liaochang (A)" <liaochang1@huawei.com>
In-Reply-To: <87sfgsvmnj.fsf@all.your.base.are.belong.to.us>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
Precedence: bulk


在 2023/1/3 2:04, Björn Töpel 写道:
> Chen Guokai <chenguokai17@mails.ucas.ac.cn> writes:
> 
>> From: Liao Chang <liaochang1@huawei.com>
> 
>> diff --git a/arch/riscv/kernel/probes/opt.c b/arch/riscv/kernel/probes/opt.c
>> index a0d2ab39e3fa..258a283c906d 100644
>> --- a/arch/riscv/kernel/probes/opt.c
>> +++ b/arch/riscv/kernel/probes/opt.c
>> @@ -271,15 +271,103 @@ static void find_free_registers(struct kprobe *kp, struct optimized_kprobe *op,
>>  	*ra = (kw == 1UL) ? 0 : __builtin_ctzl(kw & ~1UL);
>>  }
>>  
>> +static bool insn_jump_into_range(unsigned long addr, unsigned long start,
>> +				 unsigned long end)
>> +{
>> +	kprobe_opcode_t insn = *(kprobe_opcode_t *)addr;
>> +	unsigned long target, offset = GET_INSN_LENGTH(insn);
>> +
>> +#ifdef CONFIG_RISCV_ISA_C
>> +	if (offset == RVC_INSN_LEN) {
>> +		if (riscv_insn_is_c_beqz(insn) || riscv_insn_is_c_bnez(insn))
>> +			target = addr + rvc_branch_imme(insn);
>> +		else if (riscv_insn_is_c_jal(insn) || riscv_insn_is_c_j(insn))
>> +			target = addr + rvc_jal_imme(insn);
>> +		else
>> +			target = 0;
>> +		return (target >= start) && (target < end);
>> +	}
>> +#endif
>> +
>> +	if (riscv_insn_is_branch(insn))
>> +		target = addr + rvi_branch_imme(insn);
>> +	else if (riscv_insn_is_jal(insn))
>> +		target = addr + rvi_jal_imme(insn);
>> +	else
>> +		target = 0;
>> +	return (target >= start) && (target < end);
>> +}
>> +
>> +static int search_copied_insn(unsigned long paddr, struct optimized_kprobe *op)
>> +{
>> +	int i =  1;
>> +	unsigned long offset = GET_INSN_LENGTH(*(kprobe_opcode_t *)paddr);
>> +
>> +	while ((i++ < MAX_COPIED_INSN) && (offset < 2 * RVI_INSN_LEN)) {
>> +		if (riscv_probe_decode_insn((probe_opcode_t *)paddr + offset,
>> +					    NULL) != INSN_GOOD)
> 
> If the second argument is NULL, and the insn is auipc, we'll splat with
> NULL-ptr exception.

Good catch, it is my fault to ignore the access to second argument in macro RISCV_INSN_SET_SIMULATE.

> 
> Hmm, probe_opcode_t is u32, right? And GET_INSN_LENGTH() returns 4 or 2
> ...then the pointer arithmetic will be a mess?

Hmm, This pointer arithemtic does make no sense here, i had debugged this function on QEMU step by step,
and it work well. Anyway, i will go through this function again, thanks.

> 
> 
> Björn

-- 
BR,
Liao, Chang